fix(matmul): count the GEMM output tile in auto_tile's VRAM budget#116
Open
boskodev790 wants to merge 1 commit into
Open
fix(matmul): count the GEMM output tile in auto_tile's VRAM budget#116boskodev790 wants to merge 1 commit into
boskodev790 wants to merge 1 commit into
Conversation
fix / bug lane — CPU-safe validation only, no GPU scorecard. `_tile_workspace_bytes_per_elem` modeled the per-(i,j,k)-step working set as `acc + 2 operand tiles`, but `_gemm_tiled_sync` also holds the freshly allocated `prod = backend.matmul(a_dev, b_dev)` live at the same time (torch.bmm cannot write into an operand), so four T×T tiles coexist: acc + A + B + prod. The missing term is one operand-sized tile (the output, in the compute dtype). Undercounting by 4/3 makes auto_tile pick T ~= sqrt(16/12) ≈ 1.15x too large, so the real per-step working set is ~1.33x the intended vram_fraction of free memory — at a valid vram_fraction >= 0.75 the tiled/out-of-core path can OOM on exactly the huge-N runs tiling exists to enable. Same class as the operand-upcast under-budget (zeokin#74), on the still-missing GEMM output term. Fix: budget the output tile too — `acc_dtype.itemsize + 3 * _tile_operand_bytes`. Updates the fp16 workspace assertions (12->16, 6->8) and adds a regression test asserting the output tile is counted for fp32 / fp16+acc / fp16-raw. Validation: uv run python -m strategy.smoke; uv run --extra test python -m pytest tests/ strategy/tests/ eval/tests/ -q (78 passed, 23 skipped). Fixes zeokin#95
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— corrects repository behavior. CPU-safe validation only; no GPU scorecard (does not enter the GPU queue).Problem
auto_tilesizes the tile edgeTso the per-(i, j, k)working set fitsvram_fractionof free VRAM, using_tile_workspace_bytes_per_elem. That model counted only three T×T tiles — the accumulator plus the two operand tiles:But
_gemm_tiled_syncalso holds a fourth T×T tensor live at the same time — the freshly allocated product frombackend.matmul(torch.bmmcannot write its result into either operand), so at each k-stepacc,a_dev,b_devandprodcoexist. The missing term is one operand-sized tile (the output, produced in the compute dtype).Per element (modelled vs actual): fp32
12vs16; fp16+accumulate_fp3212vs16; fp16 raw6vs8. SinceT ∝ 1/√(per_elem), undercounting by 4/3 makesauto_tilepickT ≈ √(16/12) ≈ 1.15×too large, so the real working set is≈1.33×the intendedvram_fraction. At a still-validvram_fraction ≥ 0.75the overshoot pushes the per-step working set past 100% of free VRAM and the tiled/out-of-core path OOMs — on exactly the huge-N runs tiling exists to enable. Same class of under-budget as #74 (operand upcast), on the still-missing GEMM output term.Fix
Budget the output tile as well:
Only
matmul/gemm.py(the model + its docstrings) andtests/change — no scorer/eval/docs/CI paths.Tests
12 → 16,6 → 8).test_tile_workspace_counts_the_bmm_output_tile(regression for [bug] auto_tile's per-step VRAM model omits the GEMM output tile, over-sizing T by ~15% and eroding the OOM margin #95): asserts the model equalsacc + 3 * operandand is exactly one operand tile larger than the old 3-term model, for fp32 / fp16+acc / fp16-raw. It fails on the pre-fix code and passes after.Fixes #95