PEtab icon indicating copy to clipboard operation
PEtab copied to clipboard

WIP: Specification of Timecourses

Open fbergmann opened this issue 1 year ago • 1 comments

This draft PR adds the descriptions from the Time course specification to the release/2.0.0 branch.

preview site is: https://petab--581.org.readthedocs.build/en/581/documentation_data_format.html#timecourses-table

fbergmann avatar May 22 '24 09:05 fbergmann

Thx Frank.

Maybe #541 and #542 can also be addressed here.

dweindl avatar May 23 '24 09:05 dweindl

Resolved merge conflicts.

This needs to be updated to match the new mostly-agreed-upon format in https://github.com/PEtab-dev/PEtab/issues/586

  • [x] changed conditions table
  • [x] changed experiments table
  • ~~[ ] add examples from google doc - to be updated to the new format~~
  • [x] update v2/gfx/petab_scope_and_files.svg: changed names, changed columns, match color of conditions+experiments table; add mapping table
  • [x] v2/gfx/petab_scope_and_files.svg / v2/gfx/petab_files.svg : smaller SBML logo / indicate alternatives

dweindl avatar Dec 05 '24 12:12 dweindl

Thanks for the feedback.

dweindl avatar Feb 11 '25 08:02 dweindl

In the timecourses, what we essentially have are event assignments triggered at t=t_trigger​. All event assignments must be executed simultaneously—this is not a step-by-step process. Instead, the model state must transition to the new state all at once. Any mathematical expressions that depend on updated parameters, compartments, or species must be re-evaluated accordingly. In other words, all changes are applied in a single step, and the AST (Abstract Syntax Tree) affected by these changes is fully re-evaluated. There is no predefined execution order—it is a single transformation of the model state.

Therefore, the claim that "compartment has priority" is incorrect and does not make sense. The underlying issue is that PETab changes are not clearly defined and have been incorrectly implemented by simulators. As seen in cases where changes affect both compartment volumes and concentrations, many simulators apply these changes sequentially, which is incorrect. Instead, all triggered changes must be applied in a single step. This clarification should also be explicitly stated for changes applied outside of timecourses.

Additionally, in SBML, there is a flag called useValuesFromTriggerTime, which allows the use of values before the assignments. This is likely what you are referring to:

The time of assignment is not affected by the Event attribute useValuesFromTriggerTime—that attribute affects the time at which the EventAssignment’s math expression is evaluated. In other words, SBML allows decoupling the time at which the variable is assigned from the time at which its value expression is calculated.

matthiaskoenig avatar Feb 19 '25 08:02 matthiaskoenig

In SBML, all ListOf constructs are unordered lists. This means an SBML model can be modified by shuffling any ListOf elements, such as ListOfEventAssignments, without affecting the numerical results of a simulation. In other words, there is no inherent order in these lists, nor are there execution priorities—except in cases where ordering is necessary, in which attributes like priority have been introduced to dictate the execution of multiple Events.

The same principle should apply when multiple changes are made in PETab: all changes should be applied simultaneously. If ordering is required, an explicit order or priority attribute must be introduced. If mathematical expressions need to reference values from the end of the previous time span, a useValuesFromTriggerTime mechanism is necessary. For cases where a mix of behaviors is needed, an Event-like construct could be introduced, allowing assignments to be grouped and prioritized.

There is a reason SBML Events are complex. Either we adopt a full event-based approach where changes can be triggered after each time span, or we adhere to a simpler default approach where all changes are applied simultaneously.

One concern is that specifications often end up reflecting existing implementations rather than defining a general, correct solution. This appears to be happening here, as most PETab implementations currently apply changes sequentially. This approach is likely incorrect, but since the specification does not clearly define the intended behavior, there is uncertainty. Moreover, it does not align with what users of SBML models would expect.

matthiaskoenig avatar Feb 19 '25 09:02 matthiaskoenig

since the specification does not clearly define the intended behavior, there is uncertainty

Right. That definitely needs to be addressed.

what users of SBML models would expect

To be honest, I am not sure for how many SBML users the users' expectations match the SBML standard when it comes to event assignments. I am still regularly confused when it comes to simultaneous changes in compartment sizes and concentrations. That was one reason why we considered different options, such as the compartment-change-first approach. I'd find some explicit priority the most intuitive and least error-prone. This would in my opinion also cause the least issues with non-SBML models, where the interpretation as dependent/independent quantity is maybe less clear.

dweindl avatar Feb 26 '25 16:02 dweindl

The main issue with establishing a fixed order for updating multiple variables (such as compartments, species, or parameters) is that mathematical calculations dependent on these variables could produce unexpected results depending on the update sequence.

For example, consider a compartment V and a concentration C. Any calculation that relies on both V and C may yield different outcomes depending on the update order. Suppose we start with V=1 and C=1, then update both to V=10 and C=10. If there is a calculation dependent on the relationship between V and C, the result will vary based on the order in which they are updated. For instance, an event triggered when V>C will only occur if V is updated before C. If C is updated first or both are updated simultaneously, the event may not trigger, leading to inconsistencies.

In other words, updating variables sequentially in a fixed order can lead to intermediate steps that place the model in an undefined state. This issue persists regardless of the specific order. Therefore, it is essential to either update all variables synchronously or clearly define and follow a predetermined update sequence for each variable to maintain a consistent model state (simply updating compartments before species does not resolve the issue).

The issue with the concentrations is just one of such a problem because species concentrations depend on their compartment volumes, so there is a math dependency linking C and V which depends on the order. But there can be many such dependencies in the math of the model.

matthiaskoenig avatar Feb 26 '25 22:02 matthiaskoenig

Okay, fair point. The intermediate model states would be a problem.

dweindl avatar Feb 27 '25 07:02 dweindl

The suggestion for the reinitialization looks good to me. I could separate step 2 into "petab changes" and "model changes", if you agree. Here's a suggestion, that also includes some other details that we discussed.

All changes defined in the condition table are applied in two consecutive phases, similar to events in SBML. The changes take precedence over other changes that may be encoded in the model, which occur in a third phase.

  1. Evaluation of targetValues [...]

  2. Assignment of targetValues to targetIds
    All evaluated targetValues are simultaneously assigned to their corresponding targetIds. It is invalid to apply more than one assignment to a single targetId simultaneously.

  3. Update of Derived Variables, Events and Finalization [...] All other changes encoded in the model are now applied as well. For example, this includes SBML events. The model state for this timepoint is the model state after all of these changes, at the end of this phase.

dilpath avatar Mar 17 '25 21:03 dilpath

separate step 2

Thanks, I incorporated your suggestion with some modifications.

dweindl avatar Mar 18 '25 07:03 dweindl

@fbergmann : Does the current state work for you?

dweindl avatar Mar 20 '25 12:03 dweindl

Hi all, I just realized that for steady state simulations, i.e. experiments with time=-inf I don't completely understand how this would work with models which have time-dependent events.

Currently it reads; "Any event triggers defined in the model must also be checked during this pre-simulation."

  • it should probably be specified that the model should be simulated to steady state from time=0.
  • it should be mentioned that models which are simulated to steady state should not have any time dependent triggers (as I understand it this is not well defined; and steady state solvers properly don't handle this correctly). Also if there are time dependent events one would want to start with time!=0 to pre-equilibration which is currently not possible.

So not clear to me how things work with "-Inf" time and time-triggered events.

matthiaskoenig avatar Mar 20 '25 18:03 matthiaskoenig

  • it should probably be specified that the model should be simulated to steady state from time=0.

Agreed that it would be good to state that explicitly. I'd say from $t_0=0$, unless the model has specified some other $t_0$.

  • it should be mentioned that models which are simulated to steady state should not have any time dependent triggers (as I understand it this is not well defined; and steady state solvers properly don't handle this correctly). Also if there are time dependent events one would want to start with time!=0 to pre-equilibration which is currently not possible.

This is not only a problem with time-triggered events, but with any event assignments that are executed after the initial timepoint. If the model has such constructs, directly solving for $dx/dt = 0$ usually won't work. Not sure if this needs to be stated explicitly. If users don't want their events triggered during the steady state simulation, they can simply switch them off by including some indicator variable in the trigger function.

dweindl avatar Mar 20 '25 18:03 dweindl