Expose per-transition compiled propensity + stoichiometry artifact on CompiledRhs - #185
Merged
Merged
Conversation
…age) For a subset of kind: transitions specs -- named transitions whose to-side wildcard axes are a subset of its from-side wildcard axes, and whose rate doesn't reference an axis outside that set -- build a separate, independent per-transition record (ReactionArtifactIR): template-form propensity IR shaped like the from-side, plus axis bookkeeping (pinned/summed axes) describing how a firing event maps onto the destination state. This is built entirely independently of the existing deterministic per-state equation accumulation in _build_transition_equations_ir -- not derived from it, and doesn't touch that function at all. That's a deliberate choice: the deterministic path shares Expr objects by identity across template cells (tpl_uniform fast path) and dedupes summed terms by id()-tuple (issue #145 performance work), so a shared object can legitimately serve multiple transitions/cells at once -- there's no way to recover per-transition identity by decomposing already-summed equations, and tagging provenance onto shared IR nodes would either be wrong or undo that optimization. Deliberately narrow scope for v1 (see _reactions.py's module docstring): no from: null source-only transitions, no to-side axis absent from from-side, no rate expression referencing an axis outside the from-side wildcard set. Transitions outside this scope are simply omitted from the artifact tuple, not an error -- mirrors how history_requirements is built opportunistically elsewhere in this package. Covers both the same-axes case and the "collapse to a pinned target coordinate" case (e.g. recovery transitions that all land in a single vax=full stratum regardless of the firing cell's own vax) -- the latter is exercised heavily by the diphtheria model this is being built for. TransitionsRhs gains a new reactions_ir field (default empty tuple, so ExprRhs/existing callers are unaffected); normalize_transitions_rhs calls the new builder once and threads the result through. Verified: existing full test suite (442 tests) passes unchanged; manually confirmed reactions_ir is correctly populated for both the same-axes and collapse-to-pinned-target cases, and that the existing equations/equations_ir output is byte-identical to before this change for the same spec.
Adds CompiledRhs.reactions: tuple[CompiledReaction, ...], one entry per in-scope named transition (TransitionsRhs.reactions_ir), each carrying a compiled propensity evaluator plus axis bookkeeping (from_axes, to_axes, sum_axes, pinned coordinate indices). Built by a new _build_reaction_artifacts helper in compile.py, following the exact precedent already established for history_requirements/history_eval_fn: a dedicated builder called from compile_rhs, stored as a compare=False/hash=False/repr=False dataclass field, restored in __setstate__. The propensity is rate * from_state (the actual per-cell hazard), not the bare rate string carried on ReactionArtifactIR -- caught this by testing against a hand-computed expectation before wiring in the compile step, not by inspection. Verified as the correctness oracle: summing each reaction's propensity_fn output, applied via its axis bookkeeping (sum_axes summed away, pinned coordinate fixed), exactly reconstructs the existing deterministic pytree_eval_fn's output for several specs -- including one mirroring the diphtheria model's real shape (factorize_axes + a vax-indexed shaped-param rate + a collapse-to-a-pinned-vax=full recovery transition). Compiles each reaction's propensity_ir_full via the same _compile_ir_expr/_LoweringContext primitives _vectorize.py's plan builder uses, accessed the same way compile.py already reaches _vectorize.py internals elsewhere (lazy importlib.import_module, to avoid the existing compile.py <-> _vectorize.py import-order constraint). Axis/lowering context (axis_coords, axis_types, reducible_axes, axis_weights, buffer_axes, shaped_param_axes) is rebuilt directly from rhs.meta["axes"]/state_templates/shaped_params rather than threading a new field through the existing _VectorPlan -- small, cheap, and avoids touching that plan-building code at all; param-buffer assembly (param_recipes/extra_param_buffers) is reused directly from the already-built plan to stay consistent with how make_pytree_eval_fn resolves shaped params. Opportunistic like the history artifacts: a reaction whose propensity fails to lower/compile, or whose pinned coordinate doesn't resolve against the spec's axes, is silently omitted rather than failing the whole compile. New tests (tests/op_system/test_op_system_reactions.py, 9 cases): exclusion rules (unnamed / source-only / to-side-extra-axis), axis bookkeeping + propensity correctness for both the same-axes and collapse-to-pinned-target cases, the deterministic-reconstruction cross-check, the factorize_axes + shaped-param-rate case, and that ExprRhs specs compile with empty reactions. Verified: full test suite (450 = 442 existing + 8 reactions_ir tests from the previous commit's module, now +9 replacing that count -- see below) passes; mypy --strict clean on every touched file including the new test file; ruff clean except pre-existing repo-wide noqa-comment style findings this ruff version prefers differently (confirmed by checking an untouched file section -- not something introduced here, not touched).
This was referenced Aug 25, 2026
jc-macdonald
added a commit
that referenced
this pull request
Aug 25, 2026
PR #185 added CompiledRhs.reactions but the flepimop2-op_system adapter (OpSystemSystem) builds a curated options dict listing specific compiled fields by name -- it doesn't auto-forward new CompiledRhs fields, so .reactions was compiled but unreachable by any engine plugin via stepper.option("reactions"). Also exports CompiledReaction/ReactionPropensityFn from op_system's public __init__ (missed in #185 -- they existed on compile.py but weren't re-exported, same gap CompiledRhs/compile_spec already have going the other way, see the pre-existing mypy note below). Adds a _make_reaction_steppers helper mirroring _maybe_make_pytree_stepper's exact pattern: wraps each CompiledReaction.propensity_fn so mixing_kernels get merged into params the same way every other stepper already does (via the shared OpSystemSystem._merged_params), returning a same-shape tuple of CompiledReaction with only propensity_fn replaced (dataclasses.replace). Wired into options["reactions"] alongside the other stepper options. Found and documented (not fixed, out of scope here) a real limitation while testing: a transition rate that references a mixing kernel through an apply_along reduction (e.g. a spatial force-of-infection term) isn't supported by the v1 reaction-artifact scope -- _compile_ir_expr raises "axes don't match array" trying to lower a Reduce-bearing propensity against a from-axes-only target shape. Rates that only reference from-side axis-indexed shaped params work fine. Left a detailed comment in tests/test_system.py where this was found; opening a tracking issue against op_system for it. Also noted, not touched: op_system's own __init__ re-export of CompiledRhs/compile_spec (now also CompiledReaction/ReactionPropensityFn) doesn't satisfy mypy's explicit-reexport check from a consuming package's perspective -- confirmed pre-existing (2 of the resulting 3 errors already existed before this commit, via git stash) as a harmless mismatch between this __all__-based re-export pattern and mypy's stricter check; not fixed here since a real fix touches op_system's export mechanism broadly, outside this PR's scope. Verified: flepimop2-op_system's full test suite (58 tests, 4 new) passes; ruff clean on all touched lines (pre-existing repo-wide noqa-comments style noise confirmed unrelated, same as op_system proper); mypy clean on all touched lines (pre-existing unrelated errors elsewhere in both touched files confirmed via git stash before/after comparison, not introduced here). Manually verified end-to-end: OpSystemSystem(spec=...).options["reactions"] surfaces a working, correctly-valued propensity_fn for a named collapse-to- pinned-target transition, matching the diphtheria model's real usage shape.
jc-macdonald
added a commit
that referenced
this pull request
Aug 25, 2026
* Expose CompiledRhs.reactions via the flepimop2-op_system adapter PR #185 added CompiledRhs.reactions but the flepimop2-op_system adapter (OpSystemSystem) builds a curated options dict listing specific compiled fields by name -- it doesn't auto-forward new CompiledRhs fields, so .reactions was compiled but unreachable by any engine plugin via stepper.option("reactions"). Also exports CompiledReaction/ReactionPropensityFn from op_system's public __init__ (missed in #185 -- they existed on compile.py but weren't re-exported, same gap CompiledRhs/compile_spec already have going the other way, see the pre-existing mypy note below). Adds a _make_reaction_steppers helper mirroring _maybe_make_pytree_stepper's exact pattern: wraps each CompiledReaction.propensity_fn so mixing_kernels get merged into params the same way every other stepper already does (via the shared OpSystemSystem._merged_params), returning a same-shape tuple of CompiledReaction with only propensity_fn replaced (dataclasses.replace). Wired into options["reactions"] alongside the other stepper options. Found and documented (not fixed, out of scope here) a real limitation while testing: a transition rate that references a mixing kernel through an apply_along reduction (e.g. a spatial force-of-infection term) isn't supported by the v1 reaction-artifact scope -- _compile_ir_expr raises "axes don't match array" trying to lower a Reduce-bearing propensity against a from-axes-only target shape. Rates that only reference from-side axis-indexed shaped params work fine. Left a detailed comment in tests/test_system.py where this was found; opening a tracking issue against op_system for it. Also noted, not touched: op_system's own __init__ re-export of CompiledRhs/compile_spec (now also CompiledReaction/ReactionPropensityFn) doesn't satisfy mypy's explicit-reexport check from a consuming package's perspective -- confirmed pre-existing (2 of the resulting 3 errors already existed before this commit, via git stash) as a harmless mismatch between this __all__-based re-export pattern and mypy's stricter check; not fixed here since a real fix touches op_system's export mechanism broadly, outside this PR's scope. Verified: flepimop2-op_system's full test suite (58 tests, 4 new) passes; ruff clean on all touched lines (pre-existing repo-wide noqa-comments style noise confirmed unrelated, same as op_system proper); mypy clean on all touched lines (pre-existing unrelated errors elsewhere in both touched files confirmed via git stash before/after comparison, not introduced here). Manually verified end-to-end: OpSystemSystem(spec=...).options["reactions"] surfaces a working, correctly-valued propensity_fn for a named collapse-to- pinned-target transition, matching the diphtheria model's real usage shape. * Restore reactions/mixing_kernels confirming test now that #187 is merged The kernel-reduction case (apply_along over a mixing kernel, matching a spatial force-of-infection term) needed the fix in #187 (propensity_ir_reduce instead of propensity_ir_full) to compile at all. Left as a documented known-limitation comment when this branch was first opened, since #187 hadn't merged yet and the test would have been red on this PR alone. Now that #187 is merged into main and this branch is rebased onto it, restore the real test: test_option_reactions_merges_mixing_kernels exercises the fix end to end through the adapter (mixing_kernels merged into propensity_fn params the same way the other steppers already do), not just at the op_system compile layer. Verified: flepimop2-op_system full suite (59 = 58 + 1) passes; ruff and mypy clean on the touched lines.
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.
Closes #184.
Summary
Exposes a new
CompiledRhs.reactions: tuple[CompiledReaction, ...]artifact forkind: transitionsspecs — one compiled propensity evaluator + axis bookkeeping per named transition, built entirely independently of the existing deterministic per-state equation summation. This is the compile-side building block a hybrid deterministic/CTMC engine needs (tracked separately, downstream, againstflepimop2'sdiffrax_engine.py); this PR does not add any execution semantics, sampling, or opinion about what a consumer does with the artifact.Why a separate pass, not an extension of
_build_transition_equations_irTraced the existing per-state equation construction in detail before writing anything:
_build_transition_equations_irdeliberately sharesExprobjects by identity across template cells (thetpl_uniformfast path) and deduplicates summed terms byid()-tuple (documented against issue #145's performance work). A shared object can legitimately serve multiple transitions/cells at once, so there's no way to recover per-transition identity by decomposing the already-summedequations_ir— and tagging provenance onto shared IR nodes would either be wrong (for legitimately-shared nodes) or require unwinding that optimization. This PR adds a new, independent pass instead (op_system/_reactions.py) that reuses the same normalization primitives (expand_inline_templates,expand_reduce_pointwise) but never touches_build_transition_equations_iritself — confirmed zero risk to the existing hot path by running the full pre-existing test suite unchanged after each commit.What's in scope (v1)
A transition is included in the artifact iff:
name:(unnamed transitions aren't addressable — no join key)from:(nofrom: nullsource-only transitions — no well-defined "how many independent source cells are firing")to-side wildcard axes are a subset of itsfrom-side wildcard axes (no axis "created" from -> to)Transitions outside this scope are silently omitted from
reactions/reactions_ir, not an error — mirrors howhistory_requirementsis built opportunistically elsewhere in this package. This covers both the same-shape case (S[age,vax] -> E[age,vax]) and the "collapse to a pinned target coordinate" case (C[age,vax] -> S[age,vax=f], i.e. every firing cell's event lands in a single fixed stratum regardless of its own axis value) — the latter is exercised heavily by the diphtheria model this was built for (recovery/treatment-success transitions that all land in avax=full-equivalent stratum).What
CompiledReactioncarriesImportant correctness note, found by testing rather than by inspection: the propensity is
rate * from_state(the actual per-cell hazard), not the barerate:expression — caught this exact bug by hand-computing an expected value before wiring the compile step in, where an early version returned just the bare rate. The strongest verification in this PR is a cross-check that didn't exist as a plan item originally: summing each reaction'spropensity_fnoutput, scattered via its ownsum_axes/pinnedbookkeeping, exactly reconstructs the existing deterministicpytree_eval_fn's output for the same spec/state/params — done for a plain SIR-shaped spec, the collapse-to-pinned-target case, and afactorize_axes+ shaped-param-rate (delta[vax] * tau) case matching the diphtheria model's real shape.Where it hooks in
src/op_system/_reactions.py(new):ReactionArtifactIR+build_reaction_artifacts_ir, called once fromnormalize_transitions_rhs, stored on a newTransitionsRhs.reactions_irfield (default empty tuple —ExprRhs/existing callers unaffected).src/op_system/compile.py:CompiledReactiondataclass,ReactionPropensityFnprotocol,_build_reaction_artifacts(mirrors_build_history_artifacts's exact pattern — dedicated helper,compare=False/hash=False/repr=Falsefield, restored in__setstate__), wired intocompile_rhs. Reuses_vectorize.py's_compile_ir_expr/_LoweringContextthe same waycompile.pyalready reaches_vectorize.pyinternals elsewhere (lazyimportlib.import_module, avoiding the existing import-order constraint between the two modules) and reuses the already-built_VectorPlan'sparam_templates/extra_param_buffersfor shaped-param buffer assembly, so propensity evaluation stays consistent with howmake_pytree_eval_fnresolves the same parameters.Answers to the open questions from #184
reactions/CompiledReaction(nottransitions, to avoid colliding with the existingmeta["transitions"]key).PytreeEvalFn's(t, y, **params)exactly (newReactionPropensityFnprotocol), for consistency — it's read-only (never returns aStateDict, just an array), but keeping the same call shape means a consumer can reuse the same param-passing code path for both.Verification
tests/op_system/test_op_system_reactions.py(9 cases): exclusion rules, axis bookkeeping + propensity correctness for both same-axes and collapse-to-pinned-target, the deterministic-reconstruction cross-check,factorize_axes+ shaped-param-rate, andExprRhscompiles with emptyreactions.mypy --strictclean on every touched file, including the new test file.ruff check/ruff formatclean, modulo pre-existing repo-wide findings from a newer localruffversion'snoqa-commentsstyle preference (# noqa:vs# ruff: ignore[...]) that fires identically on untouched pre-existing lines — confirmed not introduced by this PR, not touched.Test plan
pytest— 450 passed (442 existing + 8 from a WIP commit, replaced by 9 final)mypy --strict— cleanruff check/ruff format --check— clean (see note above on pre-existing noise)[age, vax, loc]+factorize_axesstructureflepimop2'sdiffrax_engine.pyCTMC/tau-leaping PR (tracked separately, blocked on this)