search

AMICI-dev / AMICI

Advanced Multilanguage Interface to CVODES and IDAS
https://amici.readthedocs.io/
Other
108 stars 30 forks source link

Execution of multiple event assignments from the same event #2430

Closed dweindl closed 5 months ago

dweindl commented 5 months ago

Multiple event assignments from the same event should be executed simultaneously. From the SBML L3V2 specs:

In other words, a single event cannot have multiple EventAssignment children assigning the same variable. (All of them would be performed at the same time, when that particular Event triggers, resulting in indeterminacy.)

However, that doesn't seem to be the case:

https://github.com/AMICI-dev/AMICI/blob/da19a550688e8232b23ace6d06722c66deb2703f/python/sdist/amici/sbml_import.py#L1670-L1673

I'd say, in the else case, that assignment should be ignored.

Example: Assuming a model with a concentration-based species S in compartment C, and an event with two event assignments – EA_S assigning to S and EA_C assigning to C. In this case, S should only change due to EA_S, but not due to the volume change caused by EA_C. Currently, first EA_S is applied, then EA_C.

Correct?

FFroehlich commented 5 months ago

https://github.com/AMICI-dev/AMICI/blob/da19a550688e8232b23ace6d06722c66deb2703f/python/sdist/amici/sbml_import.py#L1670-L1673

I'd say, in the else case, that assignment should be ignored.

Example: Assuming a model with a concentration-based species S in compartment C, and an event with two event assignments – EA_S assigning to S and EA_C assigning to C. In this case, S should only change due to EA_S, but not due to the volume change caused by EA_C. Currently, first EA_S is applied, then EA_C.

Correct?

Yes, sounds correct.

dweindl commented 5 months ago

Might have been correct before. I don't quite understand why, yet. But changing that leads to failures in:

dweindl commented 5 months ago

From elsewhere in the SBML spec (sec 4.6.8):

There is one exception: if the species’ quantity is determined by an AssignmentRule, RateRule, AlgebraicRule, or an EventAssignment and the species has the attribute value hasOnlySubstanceUnits=“false”, it means that the concentration is assigned by the rule or event; in that case, the amount must be calculated when the compartment size changes.

It's not exactly intuitive and quite possibly a common source of error, but the present implementation is correct.