Skip to content

Handle state-independent, parameter-dependent event triggers without root-finding#2913

Merged
dweindl merged 1 commit intoAMICI-dev:mainfrom
dweindl:fexplicit_roots
Jul 21, 2025
Merged

Handle state-independent, parameter-dependent event triggers without root-finding#2913
dweindl merged 1 commit intoAMICI-dev:mainfrom
dweindl:fexplicit_roots

Conversation

@dweindl
Copy link
Member

@dweindl dweindl commented Jul 18, 2025

Since #2227, events that trigger at an explicit timepoint are handled without root-finding. Events with state-independent but parameter-dependent triggers were still handled with root-finding. With these changes, those event triggers will also be handled without root-finding.

This adds Model::fexplicit_roots(p, k) to compute parameter/constant-dependent roots and replaces the old list of parameter-independent roots.

Closes #2185.

@dweindl dweindl self-assigned this Jul 18, 2025
@dweindl dweindl changed the title Handle state-independent, parameter-dependent events without root-finding Handle state-independent, parameter-dependent event triggers without root-finding Jul 18, 2025
@codecov
Copy link

codecov bot commented Jul 18, 2025

Codecov Report

❌ Patch coverage is 81.65138% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.90%. Comparing base (4bfa683) to head (a884dd2).
⚠️ Report is 53 commits behind head on main.

Files with missing lines Patch % Lines
include/amici/model.h 0.00% 4 Missing ⚠️
...odel_jakstat_adjoint_py/model_jakstat_adjoint_py.h 33.33% 2 Missing ⚠️
models/model_nested_events_py/deltaxB.cpp 0.00% 2 Missing ⚠️
models/model_nested_events_py/stau.cpp 50.00% 2 Missing ⚠️
models/model_robertson_py/model_robertson_py.h 33.33% 2 Missing ⚠️
models/model_steadystate_py/model_steadystate_py.h 33.33% 2 Missing ⚠️
models/model_calvetti_py/model_calvetti_py.h 75.00% 1 Missing ⚠️
models/model_dirac_py/model_dirac_py.h 75.00% 1 Missing ⚠️
models/model_events_py/model_events_py.h 75.00% 1 Missing ⚠️
models/model_nested_events_py/deltaqB.cpp 0.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2913      +/-   ##
==========================================
- Coverage   79.45%   78.90%   -0.56%     
==========================================
  Files         358      362       +4     
  Lines       23044    23092      +48     
  Branches     1581     1585       +4     
==========================================
- Hits        18310    18221      -89     
- Misses       4723     4860     +137     
  Partials       11       11              
Flag Coverage Δ
cpp 74.82% <81.65%> (+0.01%) ⬆️
cpp_python 33.19% <44.03%> (+0.01%) ⬆️
petab 38.92% <34.61%> (-0.05%) ⬇️
python 73.69% <78.70%> (+0.02%) ⬆️
sbmlsuite-jax ?

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

Files with missing lines Coverage Δ
include/amici/abstract_model.h 100.00% <ø> (ø)
include/amici/model_dae.h 75.00% <ø> (ø)
include/amici/model_ode.h 100.00% <ø> (ø)
include/amici/serialization.h 95.19% <100.00%> (ø)
include/amici/sundials_matrix_wrapper.h 85.71% <ø> (ø)
models/model_calvetti_py/explicit_roots.cpp 100.00% <100.00%> (ø)
models/model_dirac_py/explicit_roots.cpp 100.00% <100.00%> (ø)
models/model_events_py/explicit_roots.cpp 100.00% <100.00%> (ø)
models/model_nested_events_py/deltasx.cpp 100.00% <100.00%> (ø)
models/model_nested_events_py/deltax.cpp 100.00% <100.00%> (ø)
... and 21 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dweindl dweindl force-pushed the fexplicit_roots branch 5 times, most recently from 3b288ea to 56d7f1f Compare July 18, 2025 20:26
@dweindl
Copy link
Member Author

dweindl commented Jul 18, 2025

This breaks/fixes the root-after-reinitialization error example 😢 .

Any ideas for other small benchmark collection models where this error can easily be triggered?

EDIT: okay, sbml-semantic-00885 could be used, but there the error is fixed by exactly the opposite of what the error message suggests 🙈
-> #2914

dweindl added a commit to dweindl/AMICI that referenced this pull request Jul 20, 2025
The updated discontinuity handling in AMICI-dev#2913 will break the current root-after-reinitialization error example.
Change to a new example based on an SBML test suite case.

Also update an error message and clean up the notebook.
github-merge-queue bot pushed a commit that referenced this pull request Jul 20, 2025
The updated discontinuity handling in #2913 will break the current root-after-reinitialization error example.
Change to a new example based on an SBML test suite case.

Also update an error message and clean up the notebook.
@dweindl dweindl force-pushed the fexplicit_roots branch 3 times, most recently from d587a62 to d588b61 Compare July 21, 2025 06:05
…root-finding

Since AMICI-dev#2227, events that trigger at an explicit timepoint are handled without root-finding.
Events with state-independent but parameter-dependent triggers were still handled with root-finding.
With these changes, those event triggers will also be handled without root-finding.

Closes AMICI-dev#2185.
@dweindl dweindl marked this pull request as ready for review July 21, 2025 12:21
@dweindl dweindl requested a review from a team as a code owner July 21, 2025 12:21
@dweindl dweindl merged commit e38afde into AMICI-dev:main Jul 21, 2025
24 of 25 checks passed
@dweindl dweindl deleted the fexplicit_roots branch July 21, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle events with time-based triggers without rootfinding?

2 participants