Skip to content

fix(strategy): --data-rank help omits decaying-spectrum and misstates its default#229

Open
statxc wants to merge 1 commit into
zeokin:mainfrom
statxc:fix/data-rank-help-decaying
Open

fix(strategy): --data-rank help omits decaying-spectrum and misstates its default#229
statxc wants to merge 1 commit into
zeokin:mainfrom
statxc:fix/data-rank-help-decaying

Conversation

@statxc

@statxc statxc commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

PR kind

  • fix
  • feat

Summary

The --data-rank CLI help was inaccurate on two counts:

  • Scope: it said "rank of generated matrices when --fill lowrank", but storage.generate uses data_rank for both --fill lowrank and --fill decaying-spectrum (r = data_rank or max(1, n // 32) in each branch).
  • Default: it said default n//32, but the real default is max(1, n//32) — floored so small n (where n//32 == 0) never produces a rank-0 / all-zero matrix.

Empirically (pure NumPy, no GPU):

data_rank= 2 -> matrix_rank=2
data_rank= 8 -> matrix_rank=8
data_rank=20 -> matrix_rank=20   # for --fill decaying-spectrum

Fix

Correct the help to name both fills and the real max(1, n//32) default, and add tests pinning the contract.

Touches only strategy/cli.py and a new test — no protected paths.

Validation

uv run --extra test python -m pytest tests/test_data_rank_decaying_spectrum.py -q   # 3 passed
uv run --extra test python -m pytest tests/ strategy/tests/ eval/tests/ -q          # 284 passed, 24 skipped

Checklist

  • CPU-safe validation passed.
  • My commits do not include Co-authored-by footers for coding agents such as Cursor, Codex, Claude, Copilot, or similar tools.
  • Not a feat PR — no scorecard required.

… its default

storage.generate uses --data-rank for BOTH --fill lowrank and --fill
decaying-spectrum, and its default is max(1, n//32) (floored so small n is
never rank 0). The CLI help claimed only '--fill lowrank' and a bare 'n//32'.
Correct the help and add pure-NumPy tests pinning that --data-rank sets the
generated rank for both fills and that the default floors at 1.
@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 13, 2026
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.

1 participant