fix(mex): -ffast-math broke NaN detection in to_step_function_mex; CI perf gates report-only#201
Open
HanSur94 wants to merge 1 commit into
Open
fix(mex): -ffast-math broke NaN detection in to_step_function_mex; CI perf gates report-only#201HanSur94 wants to merge 1 commit into
HanSur94 wants to merge 1 commit into
Conversation
… perf gates report-only -ffast-math implies -ffinite-math-only, letting the compiler assume no NaNs and fold the IEEE self-compare NaN test (v == v) to constant true — including clang's IR lowering of the NEON/AVX compare intrinsics. The to_step_function_mex NaN-segment scan therefore treated every NaN segment as active: all-NaN inputs produced full step functions and NaN gaps in StateTag staircases rendered as solid steps (all 9 TestToStepFunctionMex failures on ARM64 MATLAB). - build_mex.m: append -fno-finite-math-only after every -ffast-math (6 sites) — restores NaN/Inf semantics, keeps reassociation + FMA wins - to_step_function_mex.c: scalar tails use mxIsNaN (opaque libmx call, survives any fast-math mode incl. MSVC /fp:fast); FAST-MATH CONSTRAINT documented in the header - rebuilt ARM64 binaries (FastSense + SensorThreshold copies); the refresh-mex-binaries workflow triggers on these paths and regenerates the remaining platforms - kernel audit: only this kernel used the vulnerable idiom (build_store already used isnan with a hazard comment; violation_cull uses mxIsNaN; minmax/lttb receive pre-segmented NaN-free data) - TestTagPerfRegression: wall-clock gates become report-only skips on shared CI runners (three false failures across two unrelated PRs on 2026-06-10); FASTSENSE_PERF_GATES=strict opts back in; gates stay hard on developer machines Verified R2025b: TestToStepFunctionMex 13/13 (was 9 failures), TestStateTag 18/18, flat test_statetag green; gate policy verified in simulated-CI and local modes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Summary
Third batch from the review sweep — the remaining correctness bug plus the CI flake source that hit both previous PRs.
-ffast-mathsilently broke NaN detection (correctness)-ffast-mathimplies-ffinite-math-only: the compiler assumes no NaNs exist and folds the IEEE self-compare NaN test (v == v) to constant true — including clang's IR lowering of the NEON/AVX compare intrinsics.to_step_function_mex's NaN-segment scan therefore saw every segment as active: an all-NaN input produced a full step function, and NaN gaps in StateTag staircases rendered as solid steps. This is why all 9TestToStepFunctionMexcases failed on ARM64 MATLAB (the suite is gated off on headless-Linux CI, which is how it went unnoticed; the prebuilt binaries from #134 carry the bug).Fix, belt and braces:
build_mex.mappends-fno-finite-math-onlyafter every-ffast-math(6 sites) — restores NaN/Inf semantics while keeping the reassociation/FMA/no-signed-zero wins. Rationale comment added.mxIsNaN(an opaque libmx call the optimizer can't fold — survives any fast-math mode, including MSVC/fp:fast, where there is no granular opt-out). Constraint documented in the kernel header.refresh-mex-binariesworkflow triggers on these paths and regenerates the remaining platforms.Kernel audit: only
to_step_function_mex.cused the vulnerable idiom —build_store_mex.calready usedisnan()with a comment warning about exactly this hazard,violation_cull_mex.cusesmxIsNaN, and the minmax/LTTB kernels receive pre-segmented NaN-free data by contract.Verified live (R2025b / Apple Silicon):
to_step_function_mex([1 5 10],[NaN NaN NaN],20)→ empty (was 6 elements);TestToStepFunctionMex13/13 (was 9 failures);TestStateTag18/18; flattest_statetaggreen.CI perf gates become report-only on shared runners
All five
TestTagPerfRegressionbenches gate on wall-clock measurements (relative overhead, absolute ms, micro-timing ratios). On GitHub's shared runners they produced three false failures across two unrelated PRs in one day (#197/#198 —consumerMigrationTickGate23→35% on identical code that also passed,bench_sensortag_getxytripping alongside).invokeBenchOrSkip_now converts gate trips into assumption-skips with the full measurement diagnostic whenCIis set;FASTSENSE_PERF_GATES=strictopts a job back into hard enforcement (for a dedicated/self-hosted runner), and gates remain hard on developer machines. Verified in both modes (simulatedCI=true: 3 passed / 2 report-only skips / 0 failed; local: unchanged hard gates).🤖 Generated with Claude Code