fix(matmul): budget fp16 fp32-upcast temporaries in _fits_in_core (#63)#111
Open
philluiz2323 wants to merge 1 commit into
Open
fix(matmul): budget fp16 fp32-upcast temporaries in _fits_in_core (#63)#111philluiz2323 wants to merge 1 commit into
philluiz2323 wants to merge 1 commit into
Conversation
_fits_in_core estimated in-core residency as 3*n*n*item_bytes (A+B+C), but for the default fp16 + accumulate_fp32 path _gemm_in_core keeps A,B (fp16) resident while also holding their fp32 upcasts and the fp32 product at once: 2*item + 3*acc = 16 bytes/elem, not 6 — a ~2.7x under-budget. A run could pass the in-core check and then OOM mid-matmul instead of falling back to the tiled path. Add _in_core_bytes_per_elem to size the peak in-core residency per dtype (mirroring _tile_operand_bytes on the tiled path) and use it in _fits_in_core. fp32/fp16-raw are unchanged (3*item). Add CPU-only tests for the per-elem accounting and the fits/does-not-fit boundary. Fixes zeokin#63
|
Please add a filled-in scorecard from |
1 similar comment
|
Please add a filled-in scorecard from |
|
Gate chain passed. This PR is queued for the next batched GPU evaluation window. |
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.
Fixes #63.
Summary
_fits_in_coredecides in-core vs tiled execution by estimating peak device residency as3 * n * n * cfg.item_bytes(A + B + C). For the defaultfp16 + accumulate_fp32=Truepath that under-budgets by ~2.7x, because_gemm_in_coredoes not just hold three fp16 buffers — during the matmul it keeps, all live at once:a(fp16)b(fp16)a.astype(fp32)b.astype(fp32)So the true peak is
16·n²but the check budgets6·n²(16/6 ≈ 2.67x). A run can therefore pass the in-core check and then OOM mid-multiply instead of falling back to the tiled path — the opposite of the safe behaviour the dispatcher exists to guarantee.The tiled path already accounts for this fp16 upcast (
_tile_operand_bytes/_tile_workspace_bytes_per_elem); the in-core path was simply never given the same treatment. This is the still-open bug PR #68 targeted before it was closed for staleness — rebuilt on currentmain.Fix
_in_core_bytes_per_elem(cfg)— peak in-core bytes per element, mirroring_tile_operand_byteson the tiled side:fp16 + accumulate_fp32:2*item + 3*acc= 163*item_fits_in_core. fp32/fp64/fp16-raw budgets are unchanged.6·n²estimate but not the true16·n²is now correctly judged out-of-core.Scope note
This is a VRAM-accounting correctness fix in the exact engine (
matmul/gemm.py); it adds no strategy/transform and changes no scored numeric 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 on this branch.Result (N=12000, full-rank, fp32, RTX 4090 (24 GB); baseline — no strategy added)
rsvdon full-rank — unchanged baseline; below the 0.8 floor, gated to 0 as usual)O(N^3), smartO(N^2·M)— unchangedrsvd) vs 639.73 ms exact — unchanged baselinersvd) vs 1658.13 MiB exact — unchanged baselineHuman-readable scorecard —
python -m eval --n 12000 --pairs 3on RTX 4090, fp32Reference regime is A100 (80 GB) per CONTRIBUTING; measured here on an RTX 4090 (24 GB) — device named so numbers are reproducible.
Checklist
state honestly which axis it trades. Correctness fix: makes the in-core VRAM estimate match true peak residency so fp16 runs stop OOMing; no cost axis changes, no accuracy traded.
Test plan (run on RTX 4090)
python tests/test_auto_tile_budget.py— 6/6, incl. newtest_in_core_bytes_accounts_for_fp16_upcast,test_fits_in_core_rejects_fp16_upcast_boundary,test_fits_in_core_accepts_when_true_budget_fitspython eval/tests/test_eval.py— 16/16 passpython strategy/tests/test_subspace.py— 12/12 passpython tests/test_correctness.py— 7/8 pass. The one failure,test_fp16_incore_tiled_parity, is pre-existing and unrelated: it also fails on cleanzeokin:mainon the same GPU, and this PR only changes the_fits_in_coredispatch decision, not fp16 numerics.<----ia--------e------- relabel refresh: 2026-07-07T14:23:28Z -->