Repository navigation
Conversation
This was referenced Oct 26, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3490 +/- ##
==========================================
+ Coverage 80.23% 80.29% +0.05%
==========================================
Files 331 331
Lines 23420 23495 +75
Branches 1631 1647 +16
==========================================
+ Hits 18792 18865 +73
- Misses 4622 4624 +2
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
FFroehlich
approved these changes
Oct 26, 2026
dweindl
force-pushed
the
root_masking
branch
2 times, most recently
from
October 28, 2026 19:50
f03a605 to
5ea6ce6
Compare
Event assignments that move a just-crossed root function back across zero, events moving their own explicit trigger time, and assignments exactly onto a threshold (e.g., a bouncing ball) left Heaviside variables inconsistent with the root functions, so events were missed or `piecewise` took the wrong branch. The solver now only reports root crossings that change the Heaviside variables (`CVodeSetRootDirection`/`IDASetRootDirection`, set from `h` before each `run`/`step`). This replaces the ignore-after-reinit machinery and the `ROOT_AFTER_REINIT` warning. Roots that SUNDIALS reports regardless of the direction (exact zeros right after a step) are dropped. After event assignments, root functions are compared to `h`. Root functions within their uncertainty of zero are resolved by the direction in which they move, also at the initial time and for unchanged root functions at zero. A Heaviside function that keeps switching back and forth at the same time raises an error. Adjoint sensitivities: pass the event-time terms through secondary discontinuities without event assignments; forward and adjoint sensitivities pick the event-time root by the same rule. Fixes AMICI-dev#3425. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
Fix event handling for event assignments that move a root function back across or onto its threshold, using root masking as suggested in #3439: the solver only reports root crossings in the direction that would change the Heaviside variables. Alternative to #3491 (post-hoc root filtering: the solver reports every crossing and the irrelevant ones are discarded), which has the same functionality and passes the same tests; one of them supersedes #3439. Fixes #3425.
If an event assignment moved a just-crossed root function back across zero, e.g.,
E1: at x < thr: x = 1.5 * x, the Heaviside variable kept the post-crossing value, and later crossings incremented it further (h= 2, 3, …), sopiecewiseexpressions took the wrong branch. Likewise, an event moving its own explicit trigger time,E: at time >= s: s = s + 1, fired only once, and an assignment exactly onto a threshold, e.g., a bouncing ball (E: at x <= 0: x = 0, v = -e * v) or onto another event's threshold, left stale Heaviside variables, as did an event that changes the direction of a root function sitting at its threshold.The solver now only reports root crossings that change the Heaviside variables: before each
Solver::run/Solver::step,CVodeSetRootDirection/IDASetRootDirectionare set fromh(h = 1: only decreasing,h = 0: only increasing). This replaces the ignore-after-reinit machinery from #3266 (Solver::ignore_roots_after_reinit, the root filtering inreinit_post_process_f, theROOT_AFTER_REINITwarning), since a just-crossed root can no longer be re-detected. SUNDIALS still reports a root that becomes exactly zero right after a step regardless of its direction; such roots are dropped (debug messageROOT_IGNORED) and the integration continues.With masking,
hmust be consistent with the root functions, otherwise crossings are missed. After each event assignment, the root functions are compared toh. Root functions within their uncertainty of zero, estimated from a state perturbation within the integration tolerances and from the time tolerance of root-finding, are resolved as follows: an assignment that moves a root function onto zero triggers the event (the trigger is true at zero); afterwards, the direction in which the root function moves decides (central finite difference in time). The same holds for root functions that crossed zero at the current time, for unchanged root functions at zero whose direction may have changed, and, at the start of a simulation or steady-state computation, for root functions at zero. A switch that flips back and forth at the same time, e.g.,x' = piecewise(-1, x > 0, 1)atx = 0, now fails with an error instead of looping or giving wrong results.With adjoint sensitivities, the event-time terms are now passed through secondary discontinuities without event assignments, and forward and adjoint sensitivities use the last rising root, or else the last crossing root, for the event time, so that secondary events triggered at a discontinuity with only falling roots get event-time sensitivities (not covered by a test).
Tests (
test_events.py):test_event_crosses_own_trigger_back,test_event_moves_own_trigger_time,test_event_assignment_onto_thresholdtest_bouncing_ball: event counts, trajectory, and forward/adjoint sensitivities against the analytical solutiontest_sawtooth_many_events: 1000 periods of events that reverse their own root function; event counts and forward/adjoint sensitivities against the analytical solution (fails onmainwith adjoint sensitivities)test_root_at_threshold_changes_direction(fails onmain),test_assignment_onto_other_threshold,test_trigger_leaves_threshold(after an assignment / att0),test_event_moves_own_trigger_back_within_tolerance,test_chattering_switch_failstest_root_after_reinit_direction_is_checkedis renamed totest_event_assignment_just_below_other_trigger, and theROOT_AFTER_REINITchecks are removed.The SBML semantic test suite cases with events or
piecewisegive the same results as onmain(627 passed, including forward and adjoint sensitivity checks).Comparison with post-hoc root filtering (#3491):
hare dropped.h. In Fix events crossed back or moved onto thresholds by event assignments, using post-hoc root filtering #3491, it does, and a dropped root that is exactly zero has to be deactivated via SUNDIALS internals (cv_gactive/ida_gactive), as the solver doesn't continue otherwise ("Root found at and very near t", SBML cases 00354, 00754–00756). This requireshto be up to date whenever the solver is called, whichSolver::apply_root_directionsdocuments.main: +232/−144 here, +210/−144 in Fix events crossed back or moved onto thresholds by event assignments, using post-hoc root filtering #3491.maindiffered from the one without sensitivities (−896682 vs. −888198); both branches give −888198 in all modes.Known limitations (both): The direction of a root function within its uncertainty of zero is estimated by finite differences, which is unreliable for very small slopes, and for DAEs uses the algebraic states' derivatives only approximately. An event that moves its own root function back by less than that uncertainty, e.g.,
at x >= 1e6: x = x - 0.005with default tolerances, does not trigger again, and a root function that dips below zero by less than it within a single step is not detected (as onmain). Event assignments are applied as state changes, sox = 1may result inxone rounding error off (as onmain).🤖 Generated with Claude Code