fix(config): reject a non-positive tile instead of silently miscomputing#120
Open
boskodev790 wants to merge 1 commit into
Open
fix(config): reject a non-positive tile instead of silently miscomputing#120boskodev790 wants to merge 1 commit into
boskodev790 wants to merge 1 commit into
Conversation
Config validated dtype, vram_fraction and storage but not tile. gemm.multiply picks the tile edge with `cfg.tile or auto_tile(...)`, so a non-positive tile slips through with a silently wrong outcome: 0 is falsy and quietly falls back to auto (ignoring the user's explicit request), and a negative tile is truthy and flows into _tiles(n, T), whose range(0, n, T) is empty -- the tiled loop writes no blocks, C is returned untouched (all zeros), and the run still reports a normal GFLOP/s. Validate in __post_init__ that tile is None (auto) or a positive int, mirroring the existing checks. CPU-safe test coverage added; no GPU path touched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lane:
fix/bug— CPU-safe validation only, no GPU scorecard, does not enter the GPU queue.Summary
Config.__post_init__validatesdtype,vram_fractionandstorage, but nottile.gemm.multiplyselects the tile edge withcfg.tile or auto_tile(...), so a non-positive tile passes through with a silently wrong result rather than an error:tile=0is falsy, so it quietly falls back to auto — the users explicit request is ignored._tiles(n, T), whoserange(0, n, T)is empty. The tiled(i, j, k)loop then writes no blocks,Cis returned untouched (all zeros), and the run still reports a normal GFLOP/s. A wrong answer presented as a successful computation is worse than a crash.Change
matmul/config.py— in__post_init__, rejecttilethat is notNoneand< 1, mirroring the existing validation.Noneremains the way to request auto-tiling.Tests (CPU-only, no GPU)
tests/test_config_tile_validation.py: negative and zero tiles raiseValueError;Noneand a positive tile are accepted; plus a guard documenting that the rejected values are exactly the ones that make the tiled cover empty.Verification
Touches only
matmul/andtests/— no protected paths (eval/,docs/,.github/,dashboard/).