Skip to content

fix(matmul): reject non-positive --tile in Config (#80)#110

Open
philluiz2323 wants to merge 1 commit into
zeokin:mainfrom
philluiz2323:fix/issue-80-validate-non-positive-tile
Open

fix(matmul): reject non-positive --tile in Config (#80)#110
philluiz2323 wants to merge 1 commit into
zeokin:mainfrom
philluiz2323:fix/issue-80-validate-non-positive-tile

Conversation

@philluiz2323

@philluiz2323 philluiz2323 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #80.

Summary

Config.__post_init__ validated dtype, vram_fraction and storage, but not tile. A non-positive --tile (e.g. -5) therefore passed the truthy check in gemm.multiplyT = cfg.tile or auto_tile(...) — and reached _gemm_tiled_sync. There _tiles(n, T) with T <= 0 yields an empty schedule, because range(0, n, T) is empty for a non-positive step:

>>> from matmul.gemm import _tiles
>>> _tiles(4096, -5)
[]

With no tiles, the (i, j, k) accumulation loop never runs and C — allocated with np.empty in storage.allocate — is returned uninitialised. That is silent data corruption of the exact engine that every strategy is scored against, i.e. the ground truth CCO promises.

(tile=0 was already masked to auto-pick by the or, but is equally invalid and is now rejected too.)

Fix

  • Reject tile < 1 in Config.__post_init__, mirroring the existing dtype / vram_fraction / storage validation. None still means "auto-pick T from free VRAM".
  • Add CPU-only tests for the rejected (0, -1, -5) and accepted (None, 256) cases in tests/test_auto_tile_budget.py.

Failure now surfaces immediately at construction instead of corrupting results:

>>> from matmul.config import Config
>>> Config(tile=-5)
ValueError: tile must be a positive integer or None

Scope note

This is an input-validation correctness fix in matmul/config.py. It adds no strategy/transform and changes no scored code path, so there is no accuracy/latency improvement to claim. The scorecard below is the unchanged baseline on the reference regime, included per CONTRIBUTING to show the eval pipeline runs green with this branch.

Result (N=12000, full-rank, fp32, RTX 4090 (24 GB); baseline — no strategy added)

metric value
accuracy 0.0022 (rsvd on full-rank — unchanged baseline; below the 0.8 floor, gated to 0 as usual)
time complexity normal O(N^3), smart O(N^2·M) — unchanged
latency 1104.11 ms (rsvd) vs 644.46 ms exact — unchanged baseline
VRAM usage 713.08 MiB (rsvd) vs 1658.13 MiB exact — unchanged baseline
RESULT_JSON: {"config":{"n":12000,"pairs":3,"dtype":"fp32","rank_m":1500,"fill":"random","accuracy_floor":0.8,"vram_unit":"gib","device":"NVIDIA GeForce RTX 4090 (PyTorch/CUDA)","seed":7},"complexity":{"normal":"O(N^3)","smart":"O(N^2 * M)  (M=min(N,max(64,N/8))=1500 -> ~O(N^3) when M grows with N)"},"exact":{"latency_s":0.6444553518667817,"peak_vram_mib":1658.125},"transforms":{"rsvd":{"accuracy":0.00218151448080565,"latency_s":1.104107509367168,"peak_vram_mib":713.07958984375,"flop_ratio_vs_exact":1.7716262975778547,"gated":true,"improvement":false,"score":0.0}},"ranking":["rsvd"],"best":"rsvd"}
Human-readable scorecard — python -m eval --n 12000 --pairs 3 on RTX 4090, fp32
=== eval report (n=12000, 3 couples, fp32, fill=random, M=1500, seed=7) ===
  complexity : normal O(N^3)   smart O(N^2 * M)
  exact      :   644.46 ms   peak   1658.1 MiB
  transform   accuracy      latency   peakVRAM   FLOPx        score
  -----------------------------------------------------------------
  rsvd          0.0022    1104.11ms    713.1MiB    1.8x            0  (gated: low accuracy)
  best (highest score): rsvd

Reference regime is A100 (80 GB) per CONTRIBUTING; measured here on an RTX 4090 (24 GB) — device named so numbers are reproducible.

Checklist

  • I ran the scorer on unseen couples — no hardcoding of seeds/matrices.
  • Accuracy and latency come from the same run at the same dtype. (fp32, seed 7 above)
  • This is an improvement (every cost axis down, accuracy held) or I
    state honestly which axis it trades. Correctness fix: rejects an input that silently returns uninitialised results; no cost axis changes and no accuracy is traded.
  • I named the device and dtype so a reviewer can reproduce the numbers. (RTX 4090, fp32)

Test plan (run on RTX 4090)

  • python tests/test_auto_tile_budget.py — 5/5, incl. new test_config_rejects_non_positive_tile (0, -1, -5 raise) and test_config_accepts_positive_tile_and_none (None, 256 construct)
  • python eval/tests/test_eval.py — 16/16 pass
  • python strategy/tests/test_subspace.py — 12/12 pass
  • python tests/test_correctness.py — 7/8 pass. The one failure, test_fp16_incore_tiled_parity, is pre-existing and unrelated to this change: it also fails on clean zeokin:main on the same GPU, and this PR does not touch the fp16 in-core/tiled paths it exercises.

<----ia--------e------- relabel refresh: 2026-07-07T14:23:28Z -->

@github-actions github-actions Bot added area:matmul Exact engine and tile schedules (matmul/) area:tests Correctness gates and test suites (tests/) status:needs-review Awaiting maintainer review status:needs-scorecard Feature / strategy PR is missing the reproducible eval scorecard labels Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Please add a filled-in scorecard from python -m eval (see CONTRIBUTING.md).

A negative or zero tile passed the truthy check in gemm.multiply and
reached _gemm_tiled_sync, where _tiles(n, T) with T <= 0 yields an empty
schedule: range(0, n, T) is empty. The tiled loop then never runs and C
(allocated with np.empty) is returned uninitialised — silent corruption
of the exact engine's ground truth.

Reject tile < 1 in Config.__post_init__, mirroring the existing dtype /
vram_fraction / storage validation; None still means auto-pick T. Add
CPU-only tests covering the rejected and accepted cases.

Fixes zeokin#80
@philluiz2323 philluiz2323 force-pushed the fix/issue-80-validate-non-positive-tile branch from c4f6b13 to 7c2ea4c Compare July 7, 2026 13:16
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Please add a filled-in scorecard from python -m eval (see CONTRIBUTING.md).

1 similar comment
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Please add a filled-in scorecard from python -m eval (see CONTRIBUTING.md).

@philluiz2323

Copy link
Copy Markdown
Contributor Author

Added a filled-in reference-regime scorecard from python -m eval --n 12000 --pairs 3 (RTX 4090, fp32) in the PR description, per CONTRIBUTING.md.

To set expectations honestly: this is an input-validation fix in matmul/config.py — it adds no strategy and changes no scored path, so the scorecard numbers are the unchanged baseline (exact 645.42 ms / 1658.1 MiB peak; rsvd gated at 0 as usual on full-rank). It's included to show the eval pipeline runs green on this branch.

GPU gates: test_eval 16/16, test_subspace 12/12, test_auto_tile_budget 5/5. test_correctness is 7/8 — the sole failure, test_fp16_incore_tiled_parity, reproduces on clean main (52e28b2) on the same GPU and is unrelated to this change (this PR doesn't touch the fp16 in-core/tiled paths).

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Please add a filled-in scorecard from python -m eval (see CONTRIBUTING.md).

1 similar comment
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Please add a filled-in scorecard from python -m eval (see CONTRIBUTING.md).

@github-actions github-actions Bot added status:queued-gpu Feat/strategy PR passed non-GPU gates and is queued for the next batched GPU evaluation and removed status:needs-scorecard Feature / strategy PR is missing the reproducible eval scorecard labels Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Gate chain passed. This PR is queued for the next batched GPU evaluation window.

@github-actions github-actions Bot added status:queued-gpu Feat/strategy PR passed non-GPU gates and is queued for the next batched GPU evaluation type:bug Something is incorrect or broken status:ready-non-gpu Fix/docs PR cleared non-GPU triage; maintainer review only and removed status:queued-gpu Feat/strategy PR passed non-GPU gates and is queued for the next batched GPU evaluation labels Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:matmul Exact engine and tile schedules (matmul/) 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.

[bug] non-positive --tile silently returns an uninitialised buffer as the "exact" product

1 participant