Skip to content

fix(strategy): reject non-finite spectral_alpha in storage.generate#273

Merged
zeokin merged 1 commit into
zeokin:mainfrom
philluiz2323:fix/strategy-spectral-alpha-finite
Jul 17, 2026
Merged

fix(strategy): reject non-finite spectral_alpha in storage.generate#273
zeokin merged 1 commit into
zeokin:mainfrom
philluiz2323:fix/strategy-spectral-alpha-finite

Conversation

@philluiz2323

@philluiz2323 philluiz2323 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

storage.generate passes spectral_alpha straight to _fill_decaying_spectrum, whose weights are np.arange(1, rank+1) ** -alpha, with no finiteness check:

  • alpha = nan → weights all NaN → the entire generated matrix is NaN.
  • alpha = inf → weights collapse to [1, 0, 0, …] → a "rank-R decaying-spectrum" matrix silently becomes rank 1.

The sibling data_rank argument is already validated at this same public boundary; spectral_alpha was the gap.

Reproduced (CPU, current main):

storage.generate(..., fill="decaying-spectrum", spectral_alpha=nan) -> matrix all-NaN = True
storage.generate(..., fill="decaying-spectrum", spectral_alpha=inf) -> rank (data_rank was 16) = 1

Fix

Guard spectral_alpha (finite and >= 0) inside storage.generate's decaying-spectrum branch, right next to the existing data_rank guard, so every caller of the public runner/storage API is protected.

Scope note: the --spectral-alpha CLI flag that used to feed this was reverted upstream (84b6604, revert of #232), so this PR deliberately does not touch strategy/cli.py. But storage.generate remains a public function that still accepts the spectral_alpha parameter (used by the decaying-spectrum fill), so the corruption is still reachable and the boundary still needs the guard. Single file + test.

Verification (CPU — no GPU needed)

  • python -m pytest tests/test_decaying_spectrum_alpha.pygenerate() rejects non-finite/negative alpha for decaying-spectrum, accepts finite values (output stays finite), and ignores the knob for unrelated fills
  • python -m pytest tests/test_cli_errors.py — unaffected, green (this PR no longer touches the CLI)

Scorecard

Not applicable: pure input-validation/robustness fix at the storage boundary, touching no scored path (ready-non-gpu class).

@github-actions github-actions Bot added area:strategy Smart strategies / transforms (strategy/) area:tests Correctness gates and test suites (tests/) status:needs-review Awaiting maintainer review type:bug Something is incorrect or broken status:ready-non-gpu Fix/docs PR cleared non-GPU triage; maintainer review only labels Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This PR currently has merge conflicts with the base branch. Please resolve them within 12 hours or the bot will close the PR automatically.

@github-actions github-actions Bot removed the status:ready-non-gpu Fix/docs PR cleared non-GPU triage; maintainer review only label Jul 16, 2026
@philluiz2323
philluiz2323 force-pushed the fix/strategy-spectral-alpha-finite branch from 47b3e32 to 03ba760 Compare July 16, 2026 05:16
@philluiz2323 philluiz2323 changed the title fix(strategy): reject non-finite --spectral-alpha fix(strategy): reject non-finite spectral_alpha in storage.generate Jul 16, 2026
@github-actions github-actions Bot added the status:ready-non-gpu Fix/docs PR cleared non-GPU triage; maintainer review only label Jul 16, 2026
storage.generate passes spectral_alpha straight to _fill_decaying_spectrum,
whose k**-alpha weights go all-NaN for a NaN alpha (the whole generated
matrix becomes NaN) or collapse to [1, 0, 0, ...] for Inf (a silent rank-1
'decaying-spectrum' couple). The sibling data_rank argument is already
validated at this public boundary; spectral_alpha was not. Guard it (finite
and >= 0) right next to the data_rank check.

(The --spectral-alpha CLI flag that used to feed this was reverted in
84b6604, but storage.generate is a public API that still takes the
parameter, so the boundary itself still needs the guard.) Add CPU tests:
generate() rejects non-finite/negative alpha for decaying-spectrum, accepts
finite values (output stays finite), and ignores the knob for other fills.
@philluiz2323
philluiz2323 force-pushed the fix/strategy-spectral-alpha-finite branch from 03ba760 to edf40d5 Compare July 16, 2026 05:32

@zeokin zeokin left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-finite spectral_alpha silently produced an all-NaN matrix or a rank-1 collapse -- corrupt benchmark input. Closes the library boundary the CLI already guards.

@zeokin
zeokin merged commit 620ce74 into zeokin:main Jul 17, 2026
6 checks passed
statxc added a commit to statxc/Cuda-Compute-OSS that referenced this pull request Jul 17, 2026
…merge

storage.py accumulated two duplicates when zeokin#273 (reject non-finite
spectral_alpha) landed:

1. import math appeared twice (lines 8 and 10) -- the only F811 in the shipped
   source tree; matmul/storage.py imports cleanly.

2. spectral_alpha is validated twice in generate(): a top-of-function guard and
   again inside the decaying-spectrum branch. The top guard is strictly stronger
   (it also rejects bool / non-Real) and runs first, so the in-branch re-check is
   dead code -- every bad alpha (nan/inf/-inf/<0/non-Real/bool/None) is caught up
   top, verified by direct call. Drop the re-check; move its rationale comment and
   its value-in-message (got {alpha!r}) onto the surviving guard, so the error is
   if anything more informative (it now names non-Real offenders the inner check
   never reached).

Behaviour is unchanged: same inputs rejected, same fills produced. Pure
redundancy removal, one file, no protected paths.
zeokin added a commit that referenced this pull request Jul 17, 2026
fix(strategy): drop two storage.py redundancies left by the #273 merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:strategy Smart strategies / transforms (strategy/) area:tests Correctness gates and test suites (tests/) status:needs-review Awaiting maintainer review status:ready-non-gpu Fix/docs PR cleared non-GPU triage; maintainer review only type:bug Something is incorrect or broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants