Harden measurement, CI, and schedule validation (Codex adversarial review) - #30
Merged
Conversation
Fixes the four findings from the adversarial review: 1. [high] Zero/sub-us measured time no longer manufactures fake speedups. runner.compile_and_run now rejects time <= 0 as a "measurement" error (single chokepoint for all four environments) instead of the callers clamping the divisor to 1e-9. TIME print precision bumped %.6f -> %.9f so legitimate sub-microsecond runs survive instead of rounding to zero. Dead max(.., 1e-9) clamps removed from backend_isl/multikernel/stencil/imperfect. 2. [high] CI no longer gates on hardware-dependent speed thresholds. The >2.0x / >1.0x asserts in test_distmatrix, test_mcp, test_multikernel are relaxed to "ran" (speedup > 0); legality/status/checksum are unchanged. 3. [medium] Malformed multi/stencil/imperfect schedules return "invalid" instead of aborting the search: except sets broadened to match backend_isl (ValueError, IndexError, KeyError, isl.Error). 4. [medium] tile/tile2d/tile3d/unroll factors must be positive integers; tile(i,0) / tile(i,-16) / missing factors are rejected at parse time before they become a never-advancing C loop that spins to the timeout. Regression guards added to tests/test_review_fixes.py (zero-time measurement error, malformed multi schedule -> invalid, positive-factor validation). Full suite 16/16. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Addresses all four findings from the Codex adversarial review.
[high] Zero/sub-µs time manufactured fake speedups
max(r["time"], 1e-9)turned a measured0.0(sub-µs work rounding to zero at the old%.6fprint precision) into a 1 ns run → a giant fake speedup that corrupted the optimizer's selection signal. Fixed at the single chokepoint all four environments share:runner.compile_and_runnow rejectstime <= 0as ameasurementerror. Print precision bumped%.6f → %.9fso legitimate sub-µs runs survive instead of rounding to zero. The now-dead clamps were removed frombackend_isl/multikernel/stencil/imperfect.[high] Required CI gated on hardware-dependent speedups
The required
testsworkflow ran wall-clock thresholds (test_distmatrix>2.0x,test_mcp>1.0x,test_multikernel>1.0x) that flake on shared GitHub runners. Relaxed to "ran" (speedup > 0); legality/status/checksum assertions are unchanged. (Same policy as the earlier montecarlo fix.)[medium] Malformed multi/stencil/imperfect schedules aborted the search
Those paths only caught
ValueError/KeyError, butbuild_thetacan raiseIndexError/isl.Erroron malformed-but-parseable input — aborting optimization instead of returninginvalidfeedback. Except sets broadened to matchbackend_isl:(ValueError, IndexError, KeyError, isl.Error).[medium] Validation accepted tile factors that hang the binary
tile(i, 0)andtile(i, -16)passed parsing (-?\d+), then became a C loop step of0/negative that never advances → the binary spins to the 120 s timeout, one full timeout per evaluation.tile/tile2d/tile3d/unrollsize factors are now required to be positive integers at parse time.Verification
tests/test_review_fixes.py: zero-time → measurement error, malformed multi schedule → invalid (not crash), positive-factor validation..venv/bin/python -m tests.Xacross all modules).🤖 Generated with Claude Code