Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarification/specification of order of condition changes required #557

Open
matthiaskoenig opened this issue Apr 27, 2023 · 5 comments · May be fixed by #581
Open

Clarification/specification of order of condition changes required #557

matthiaskoenig opened this issue Apr 27, 2023 · 5 comments · May be fixed by #581
Milestone

Comments

@matthiaskoenig
Copy link

Depending on the order of the changes applied for a condition the model will be in a different state. I.e. the order of the columns matters in the condition table.
E.g. a model with species S in compartment C with an initial concentration of 1.0. If you change first the Volume C and then the initial concenctration [S0], the model has different values for [S0] compared to first changing concentration and then volume.

Currently it states

Row- and column-ordering are arbitrary, although specifying conditionId first may improve human readability.

This should be changed to:

Row- and column-ordering do not have any restrictions, although specifying conditionId first may improve human readability. The parameterOrSpeciesOrCompartmentIds changes for a condition are applied in order of the columns.

See also for instance: AMICI-dev/AMICI#2062

@FFroehlich
Copy link
Collaborator

Hmm, interesting point. I would prefer to assume that volume changes always take precedence over concentration changes.

@dweindl
Copy link
Member

dweindl commented Apr 27, 2023

Agreed, that needs to be clarified.

I would prefer to assume that volume changes always take precedence over concentration changes.

This is also what would happen if an SBML model has an InitialAssignment for both the initial concentration and the respective compartment size, right? Sounds good then.

@FFroehlich
Copy link
Collaborator

SBML level 3 version 2 release 2:

The ordering of InitialAssignment objects in a model is not significant. The collection of InitialAssignment, AssignmentRule and KineticLaw objects forms a set of assignment statements that must be considered as a whole. The combined set of assignment statements should not contain algebraic loops: a chain of dependency between these statements should terminate.

@FFroehlich
Copy link
Collaborator

would propose to handle this the same way as for simultaneous assignment of volume size & concentration:

Upon such size changes, the value of the concentration or density must be recalculated from the simple relationship concentration = amount/size if the value of the concentration is needed (for example, if the species identifier appears in a mathematical formula or is otherwise referenced in an SBML construct). 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.

@matthiaskoenig
Copy link
Author

This is the same issue as in SBML Events with multiple EventAssignments based on a trigger (i.e. this has been solved already for events and the same solution should be used here).

4.12.5 EventAssignment

An “event assignment” has effect when the event is executed. (As noted above, the operation of event is divided
conceptually into two phases: the first phase when the event is triggered and the second phase when the event
is executed.)

In SBML all event assignments are performed simultaneously. The same should happen in PEtab for all the model changes for a given condition. I.e. there is no order of execution. All changes are applied at once.

@dweindl dweindl linked a pull request May 30, 2024 that will close this issue
@dweindl dweindl added this to the PEtab 2.0.0 milestone Jul 3, 2024
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 a pull request may close this issue.

3 participants