fix(convert): carry #89's stable tQSSA root into the SBML export - #94
Merged
Conversation
`_mm_formula` was the one site deliberately left out of #89, on the reading that an interchange artifact should stay a faithful transcription of the canonical literature formula. That reading does not survive the measurement: SBML MathML is evaluated by the *consumer*, in float64, so the exported law performed the same `0.5*(delta + D)` subtraction the engine had just stopped performing. With #89 in place the two sides disagree, and the gate says so. On tests/data/mm_tqssa_stiff.net (ratio ~1e7), textbook -> stable: RHS vs source model 2.17e-02 -> 0.0 (bit-for-bit) conversion gate L2/L3 fail, ok=False -> L0-L3 pass, ok=True libRoadRunner CV_ERR_FAILURE -> agrees with bngsim to 2.9e-11 parity _faithful_sbml_text REFUSED (> 1e-6 tol) -> ACCEPTED So net2sbml was emitting a model its own validator marks not-ok, that RoadRunner cannot integrate, and that the bng_parity RoadRunner oracle silently drops as unfaithful. The export now carries the same branch as `_mm_sfree_c_lines`, spelled as the L3v2 piecewise the writer already targets: sFree = piecewise(0.5*(delta + D), delta >= 0, 2*Km*S/(D - delta)) omitting only that helper's `(D - delta) > 0` degenerate guard, unreachable once delta < 0 and backstopped by the enclosing max(..., 0). The piecewise costs less than it looks. It is not a new construct here: the target is L3v2 chosen for exactly this MathML, `piecewise` is already in _EXPRTK_SUPPORTED_CALLS, and every BNGL `if()` already exports as one. It does not move L4 either, which is *already* inconclusive on MM (pinned by test_full_gate_l4_is_non_gating) because the law already contains `max`, and _validate.py lists Max and Piecewise in the same _NONSMOOTH tuple. Unlike that `max` - a genuine kink - this branch is removable: both arms agree to the last ulp at delta == 0 (measured across +/-1e-3 .. +/-1e-12, worst 1.11e-16), so no consumer's integrator gains a discontinuity. The real cost is size: 6519 -> 12121 bytes on the stiff fixture, since sFree appears twice and SBML has no `let` binding. Where the corpus lives nothing moves: at ratio ~1 and on the delta >= 0 branch the emitted law is bit-identical to the textbook one, so both corpus MM models export unchanged.
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.
Follow-up to #92 (merged), which took the stable tQSSA root everywhere except the
SBML exporter. This is that last site.
Summary
_mm_formulainpython/bngsim/convert/_sbml_writer.pywas the one sitedeliberately left out of #92, on the reading that an interchange artifact should
stay a faithful transcription of the canonical literature formula.
That reading does not survive the measurement. SBML MathML is evaluated by the
consumer, in float64 — so the exported law kept performing the same
0.5*(delta + D)subtraction that #92 had just stopped the engine performing.Once the two sides disagree, the conversion gate says so.
Measurement
tests/data/mm_tqssa_stiff.net(the fixture #92 adds,|δ|/√(4·Km·S)~ 1e7),with #92's engine fix in place:
ok=Falseok=TrueCV_ERR_FAILURE, no trajectory_faithful_sbml_text(bng_parity oracle)So
net2sbmlwas emitting a model that its own validator marks not-ok, thatRoadRunner cannot integrate, and that
parity_checks/bng_parity/net_roadrunner.pysilently drops as unfaithful — an unscored
REFERENCE_FAILEDrow rather than aloud failure.
Rate accuracy in RoadRunner across the #89 sweeps, textbook → piecewise:
Rows 2 and 5 are bit-identical — the
delta >= 0branch keeps the textbookpath exactly, so nothing moves where the corpus lives.
Change
The export now carries the same branch as
_mm_sfree_c_lines, spelled as theL3v2
piecewisethe writer already targets:It omits only that helper's
(D - delta) > 0degenerate guard, which isunreachable once
delta < 0(D >= 0forcesD - delta > 0) and which theenclosing
max(..., 0)backstops anyway.Why the
piecewiseis cheaper than it looksBoth concerns that kept this out of #92 turned out to be already-settled:
exactly this MathML (
_SBML_VERSION),piecewiseis already in_EXPRTK_SUPPORTED_CALLS, and every BNGLif()already exports as one.inconclusiveon every MM model — pinned by the existing
test_full_gate_l4_is_non_gating—since the law already contains
max, and_validate.pylistsMaxandPiecewisein the same_NONSMOOTHtuple.And the new branch is removable, unlike the
maxalready in the law: both armsagree to the last ulp at
delta == 0(measured across±1e-3 … ±1e-12, worst1.11e-16), so no consumer's integrator gains a discontinuity to resolve.
The cost that is real is size: the emitted document grows 6519 → 12121 bytes
on the stiff fixture, because
sFreeappears twice in the law and SBML has noletbinding. AfunctionDefinitionwould fix that; it is a larger structuralchange (it moves L1) and is deliberately not in this PR.
Tests
test_mm_tqssa_stiff_rhs_exact— RHS delta on the stiff fixture. Measured2.17e-02 against the textbook form, exactly 0.0 against the piecewise.
test_mm_tqssa_stiff_full_gate_passes— the end-to-end tripwire: with thetextbook root, L2 and L3 both fail and
report.okis False.Both verified to fail without the change and pass with it.
Full
python/testssuite: 2763 passed, 114 skipped. pre-commit (ruff,ruff-format, mypy) clean. libRoadRunner 2.9.2 used for the consumer checks.
Per
native-tests.yml,python/testsis covered by the pre-push hook rather thanby a workflow, so both new tests ran there (and locally) but will not appear in
this PR's checks.