ci(mir): run the Michaelis-Menten sensitivity tests, and correct the Windows note - #91
Merged
Merged
Conversation
…Windows note Two follow-ups to #87 and #88, both confined to mir.yml. test_codegen_mm_sens.py lands in the run list. #87 added the other three analytic-sensitivity files; #55's MM stage merged after it, so its 17 tests were covered only by the pre-push hook. It needs no #85 quarantine, and the reason is recorded in the header rather than left to be rediscovered: #85 fires on bngsim_codegen_output_sens, which generate_output_sens_from_model emits only for a model with at least one *function*. The MM fixtures declare none, so the symbol is absent even with emit_output_sens forced on, and the Simulator-with-sensitivity_params cases compile. Verified locally on both fixtures. c2mir already handles the arithmetic: mm_tqssa.net goes through test_mir_equivalence and test_codegen_jacobian on this job today, and the MM sensitivity block is the same closed form as the Jacobian block, emitted by the same builder. The Windows paragraph was factually wrong. It said the needs_cc cc-baseline tests skip on windows-latest and that the compiler-free MIR-vs-ExprTk check was therefore the whole of the Windows verification. Neither holds: the runner ships MinGW gcc on PATH, so `_CC = which("cc") or which("clang") or which("gcc")` resolves, nothing skips (0 skipped on every leg, measured), and test_mir_matches_cc runs and passes there. All four legs assert cc-vs-MIR bit-identity, so the job covers more than the comment claimed, not less. Replaced with what is true, plus the consequence that matters: because the runner has compilers, this job does not exercise the no-compiler-at-all path that motivates MIR in the first place. Guarding that needs a leg with the compilers taken off PATH.
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.
Two follow-ups to #87 and #88, both confined to
mir.yml.test_codegen_mm_sens.pyjoins the run list#87 added the other three analytic-sensitivity files. #55's Michaelis–Menten stage (#88) merged after it, so its 17 tests are currently covered only by the pre-push hook — the gap #87 existed to close, reopened by timing.
It needs no #85 quarantine, and the header now records why rather than leaving it to be rediscovered:
#85 fires on
bngsim_codegen_output_sens, andgenerate_output_sens_from_modelemits that only when the model has at least one function. The MM fixtures declare none, so the symbol is absent even withemit_output_sensforced on:Same reason
test_steady_state_dfdp_stops_differencingpasses under the JIT today (your note on #85). So theSimulator(..., sensitivity_params=...)cases inTestEndToEndshould compile on this backend.Corroboration that c2mir handles the arithmetic:
mm_tqssa.netalready goes throughtest_mir_equivalenceandtest_codegen_jacobianon this job, and the MM sensitivity block is the same closed form as the Jacobian block, emitted by the same builder (_mm_jacobian_groups). The ctypes/compile_rhsharness is copied fromtest_codegen_functional_dfdp.py, which passes 29/29 under the JIT.I could not run a MIR build to confirm end to end. If a leg does go red on this file, the fix is one
blocked_on_mir_jitmarker onTestEndToEnd— the same shape #87 used — and I'd want the failure text first, since it would mean #85 is broader thanbngsim_codegen_output_sens.The Windows paragraph was wrong
It claimed the
needs_cccc-baseline tests skip onwindows-latest, and that the compiler-free MIR-vs-ExprTk check was therefore the whole of the Windows verification. Neither holds:gccon PATH, so_CC = which("cc") or which("clang") or which("gcc")resolves;156 passed, current main is229 passed, 10 xfailed;test_mir_equivalence.py::test_mir_matches_cc[...]runs and passes on Windows.So all four legs assert cc-vs-MIR bit-identity on the same generated source. The comment understated the job's coverage, which is the direction that matters — someone reading it would think Windows was weakly covered and might not add a test there.
Replaced with what is true, plus the consequence worth keeping: because the runner has compilers, this job never exercises the no-compiler-at-all path that motivates MIR. Guarding that needs a leg with the compilers taken off PATH, not this one. Might be worth its own issue; I have not filed one.
Test plan
test_codegen_mm_sens.py,test_codegen_switch_condition_sens.py,test_codegen_functional_sens_rhs.py,test_codegen_functional_dfdp.py— 100 passed locally on the cc backend. The MIR legs of this PR are the real check, since the change is what they run.