[dweindl]

Fixes #3126.

Event.__init__ tries sympy.solve(trigger, t) to precompute when an event fires, so that the event can skip numerical root-finding. But AMICI encodes And/Or-combined triggers as real-valued root functions built from Min/Max (_parse_event_trigger in the SBML importer, and the PEtab v2 importer's period-start events), and sympy.solve raises NotImplementedError on those. That exception was silently swallowed, leaving _t_root empty, so every such event — including every PEtab v2 period-start event — took the slower path even where its trigger time is statically known.

solve_minmax_trigger_for_t() handles exactly that structure and nothing else. Every t-dependent leaf of the tree comes from a relational trigger atom, so it is affine in t and therefore monotonic with a single root. All leaves must run in the same direction, so that the expression crosses zero exactly once: with increasing leaves a Min (And) crosses at the latest of the per-leaf roots and a Max (Or) at the earliest; with decreasing leaves the two roles swap. Time-independent ("gating") leaves of a Min — e.g. the experiment/preequilibration indicator comparisons — become the condition of Piecewise((<root>, <all gating >= 0>), (oo, True)). For the negated counterpart, the gating leaves must be strictly positive, since a zero gating leaf keeps it at zero. If the gating condition is false for all parameter values, the event never triggers and no trigger time is returned.

A negated expression is normalised before any of that, which is load-bearing rather than cosmetic: AMICI tracks a persisted Heaviside trigger together with its negated counterpart, and the two cross zero at the same time. If only one of them resolved, the pair would end up with different classifications and _reorder_events() would separate them, corrupting the event bookkeeping. Normalising means both twins reduce to the same expression, up to the strictness of the gating condition, which doesn't change their classification.

Deliberately out of scope, all returning None and keeping today's empty-_t_root behaviour: non-affine leaves, leaves of mixed monotonicity (which may cross zero more than once), mixed Min-inside-Max nesting, and Max with a gating leaf. The tests pin each of these bail-outs so the conservative paths don't silently widen later.

No codegen or C++ changes are needed for the conditional case. sp.oo already prints as std::numeric_limits<double>::infinity(), Piecewise already has C++ codegen support, and the runtime already treats "no next explicit trigger time" as infinity and takes min(next_t_out, next_t_event) — so a trigger time that conditionally evaluates to +inf is simply never reached. Pre-/post-equilibration guards next_t_event with std::isfinite and falls back to a geometric step, so the solver clock never becomes infinite there either.

Verified end-to-end through the real PEtab v2 importer, on a problem with preequilibration plus a dynamic period starting at t=10: the period-start event's trigger time now resolves to 10 when the experiment is active and to +inf (never) otherwise, and simulated observables are unchanged.

On the PEtab v2 conversions in Benchmark-Models-PEtab#338 (34 problems), all 304 period-start events (Isensee_JCB2018, Raimundez_PCB2020, Brannmark_JBC2010, Weber_BMC2015, Zheng_PNAS2012) now have explicit trigger times; on main, none had. No other trigger changes classification. All of these periods start at t=0; with additional periods at t=5 and t=10 added to Zheng_PNAS2012, those period-start events are explicit as well.

Also resolves a TODO that referenced this issue: DEModel.has_implicit_event_assignments() rejected any state-updating event whose trigger referenced a free parameter, although only state-dependent trigger times are actually unsupported. It now allows a model's static symbols rather than only its constants. The TODO suggested switching it to has_explicit_trigger_times() instead, but that asks a different question — "can the firing time be solved for" rather than "does it depend on the trajectory" — and would have started rejecting unsolvable-but-time-only triggers such as sin(t) > 0.5. The newly admitted case was checked against exact gradients, not just accepted.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 91.11111% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.07%. Comparing base (15ca6bd) to head (9d2095c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
python/sdist/amici/_symbolic/sympy_utils.py	89.74%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3302      +/-   ##
==========================================
+ Coverage   80.06%   80.07%   +0.01%     
==========================================
  Files         319      319              
  Lines       22876    22915      +39     
  Branches     1562     1561       -1     
==========================================
+ Hits        18315    18350      +35     
- Misses       4554     4558       +4     
  Partials        7        7              

Flag	Coverage Δ
cpp	74.39% <91.11%> (+0.03%)	⬆️
cpp_python	34.84% <4.44%> (-0.06%)	⬇️
petab	48.27% <73.33%> (+0.15%)	⬆️
petab_sciml	16.87% <6.66%> (-0.04%)	⬇️
petab_sciml_benchmarks	15.43% <6.66%> (-0.04%)	⬇️
python	72.41% <91.11%> (+0.03%)	⬆️
sbmlsuite	50.16% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
python/sdist/amici/_symbolic/de_model.py	93.99% <100.00%> (-0.01%)	⬇️
...ython/sdist/amici/_symbolic/de_model_components.py	96.64% <100.00%> (+0.01%)	⬆️
python/sdist/amici/_symbolic/sympy_utils.py	96.10% <89.74%> (-2.16%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dweindl]

Update: the earlier CI failures (8 JAX PEtab v2 cases and case 0030) are resolved. The JAX And/Or printing was fixed via #3306, and case 0030 passes since PEtab-dev/libpetab-python#562 assigns a priority to the period-start event. CI is green after rebasing.

🤖 Generated with Claude Code