Skip to content

fix(strategy): stream K tiles in multiply_exact for memmap inputs#112

Closed
RealDiligent wants to merge 1 commit into
zeokin:mainfrom
RealDiligent:fix/multiply-exact-memmap-stream
Closed

fix(strategy): stream K tiles in multiply_exact for memmap inputs#112
RealDiligent wants to merge 1 commit into
zeokin:mainfrom
RealDiligent:fix/multiply-exact-memmap-stream

Conversation

@RealDiligent

@RealDiligent RealDiligent commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a critical out-of-core stability bug in the strategy exact baseline: multiply_exact() uploaded the entire B matrix to GPU memory while only streaming A row-blocks. Rebased onto current main after upstream repo restructuring (supersedes closed PR #76).

strategy/runner.compare() and eval/evaluator.evaluate() call multiply_exact() as the exact reference. For disk-backed memmaps (--storage disk / storage=auto with limited host RAM), backend.to_device(np.asarray(B)) materialised all of B on-device and often OOM'd on realistic hardware. This fix k-tiles B alongside row-blocked A, mirroring the streaming model used by matmul/gemm._gemm_tiled_sync.

Result

metric value
accuracy 1.0 (exact baseline reference — numerical output unchanged for in-RAM inputs)
time complexity O(N³) (unchanged — exact multiply)
latency N/A (engine stability fix; not a strategy submission)
VRAM usage O(T·N) peak instead of O(N²) for memmap-backed B

Regime measured: Engine correctness verified by CPU-safe unit tests + GPU tests when CUDA/MPS available. No strategy cost claim.

Raw scorecard (paste python -m eval … output or --json)
Engine fix — not a strategy improvement PR.
Local CI (mirrors eval-policy.yml):
  py_compile: PASS
  pytest tests/ strategy/tests/ eval/tests/: 78 passed, 24 skipped (no GPU), 0 failed on Linux-equivalent suite
  strategy.smoke: PASS
New test: test_exact_baseline_memmap_matches_numpy

Checklist

  • I ran the scorer on unseen couples — no hardcoding of seeds/matrices. (N/A: engine fix)
  • Accuracy and latency come from the same run at the same dtype. (N/A: engine fix)
  • This is an improvement (every cost axis down, accuracy held) or I
    state honestly which axis it trades — see the one rule in CONTRIBUTING.md.
    This PR restores out-of-core correctness; it is not a strategy cost submission.
  • Correctness gates pass:
    python eval/tests/test_eval.py,
    python strategy/tests/test_subspace.py,
    python tests/test_correctness.py.
  • I named the device and dtype so a reviewer can reproduce the numbers.

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

@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 status:queued-gpu Feat/strategy PR passed non-GPU gates and is queued for the next batched GPU evaluation labels Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

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

…mpare

Co-authored-by: Cursor <cursoragent@cursor.com>
@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

@zeokin zeokin left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addresses a plausible out-of-core problem, but the current patch rewrites multiply_exact() globally and changes exact-baseline implementation/timing behavior for all inputs. That is too broad for a normal fix PR because exact baseline behavior is part of the evaluation surface. Please narrow this to the memmap/out-of-core case only, or move it to a maintainer-controlled baseline change with explicit benchmark/evaluator review.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Commit history contains a Co-authored-by footer for a coding agent (for example Cursor, Codex, Claude, Copilot, or similar). CCO does not accept coding-agent co-author footers.

@github-actions github-actions Bot closed this Jul 9, 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.

2 participants