Update and fix PEtab parameters description#47
Conversation
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
| specified for all simulated conditions. | ||
| semicolon-delimited list of all relevant condition IDs, and an array must be | ||
| provided for all initial PEtab conditions (the first condition per PEtab v2 | ||
| experiment). For :ref:`static hybridization <hybrid_types>`, array inputs can |
There was a problem hiding this comment.
Am I correct that the user can optionally provide arrays for non-initial conditions as well? i.e. they have to provide inputs for the first condition but they can choose to provide inputs for subsequent conditions within the same experiment (or not).
There was a problem hiding this comment.
Yes. For example, they might use that condition as initial condition in another experiment.
Also for dynamic hybridization we realized that nothing is really disallowing an array input, e.g., input argument 0 might be a model specie, while input 1 might be omics data as an example.
There was a problem hiding this comment.
The semantics of specifying for only initial conditions vs. all initial conditions and some/all subsequent conditions must be made clear I think. i.e. since the NN input must be defined everywhere the NN is used in the dynamic case, is the input a piecewise-constant function that is updated whenever a new condition specifies new parameters?
There was a problem hiding this comment.
I mean for the dynamic case, does not the same rule apply as for standard PEtab. That is, if for a condition the input does not change, we simply use the value from the preceding condition?
There was a problem hiding this comment.
Yes, you're probably right, no need to specify this additionally. We could link to this for the presimulation/static NN case and say it is similar to preinitialization here: https://petab.readthedocs.io/en/latest/v2/documentation_data_format.html#v2-initialization-semantics
Maybe we should simply replace the use of presimulation with preinitialization everywhere in PEtab SciML, to match PEtab
And we could link to this for describing how inputs change during a PEtab experiment: https://petab.readthedocs.io/en/latest/v2/documentation_data_format.html#v2-reinitialization-semantics
Co-authored-by: BSnelling <branwen.snelling@crick.ac.uk>
dilpath
left a comment
dilpath
left a comment
There was a problem hiding this comment.
This time I reviewed using the visual diff feature of readthedocs, worked nicely! e.g. https://petab-sciml--47.org.readthedocs.build/47/format.html?readthedocs-diff=true&readthedocs-diff-chunk=2
Looks good! Thanks very much
| specified for all simulated conditions. | ||
| semicolon-delimited list of all relevant condition IDs, and an array must be | ||
| provided for all initial PEtab conditions (the first condition per PEtab v2 | ||
| experiment). For :ref:`static hybridization <hybrid_types>`, array inputs can |
There was a problem hiding this comment.
The semantics of specifying for only initial conditions vs. all initial conditions and some/all subsequent conditions must be made clear I think. i.e. since the NN input must be defined everywhere the NN is used in the dynamic case, is the input a piecewise-constant function that is updated whenever a new condition specifies new parameters?
| The schema is provided as a :download:`JSON schema <standard/array_data_schema.json>`. | ||
| Currently, validation is only provided via the PEtab SciML library, and does | ||
| not check the validity of framework-specific IDs (e.g. for inputs, parameters, | ||
| not check the validity of framework-specific IDs (e.g. for inputs, parameters, |
There was a problem hiding this comment.
What is the diff doing here 🤔 I guess there is a weird unicode whitespace character
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
|
Following major confusion regarding PEtab conditions and hybridization mode, I have updated the proposed PR. |
|
I will close this as I got array inputs for dynamic hybridization wrong, and it is best to wait for #52 before updating the spec. |
3 participants
Closes #41 #45
A description on priors was lacking, so added it as well.