-
Notifications
You must be signed in to change notification settings - Fork 2
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
PEtab YAML formatting #4
Comments
If I understand this point correctly, you are only here referring to network outputs? Anyhow, I think we should still allow this for two reasons. First, in the case of a single experimental condition, it is beneficial for users to avoid dealing with an additional table. Second, if the output is consistent across all conditions, it feels cumbersome to repeatedly define it in the conditions table. For greater flexibility once https://github.com/PEtab-dev/PEtab/issues/586)](https://github.com/PEtab-dev/PEtab/issues/586 is resolved, we should also support having the output in the conditions table (but there are potential ambiguities in the PEtab spec here with for example |
But you ultimately run into the same issues that are being addressed in PEtab v2. are those assignments initial conditions, constant values, time derivatives etc. I would argue that having a logical and easy to understand syntax is probably better for users than saving some tables for some use cases.
Yes waiting until the respective spec is finalized sounds good, but I thought this was already pretty close to being agreed to. |
Fair point, and in hindsight having these things explicitly stated will help in the parsing. Then, lets go with this. So how about this. We wait for PEtab-dev/PEtab#586 to be finalized (to avoid surprises). Meanwhile I will keep the mapping table like now, address the other points in this issue, and open a new issue on adapting to the new condition table format. |
sounds good! |
At this point, unless someone complains, it will have the proposed structure, with some naming changes to make things more intuitive. Daniel worked on this a bit more in the last couple of weeks.
👍 |
I identified a couple of issues with the petab files for the test cases
all observable ids have the same name as the model entity, which is afaik not valid petab
we should use format_version
2.0.0
as mapping tables are not available with1.0.0
sbml_files
to to fieldmodel_files
with dictlanguage
(sbml) andlocation
(file)mapping_tables
attribute to listmapping_files
petab.MODEL_ENTITY_ID
(instead ofioId
) andpetab.PETAB_ENTITY_ID
(instead ofioValue
). I wouldn't introduce a separatenetId
column, but rather absorb this intopetab.MODEL_ENTITY_ID
using a nomenclature such asnet1.input1
. see Proposal: Different languages for model specification PEtab-dev/PEtab#538. I am also not sure whether we should make assignments to petab variables in the mapping table (model outputs to petab variables), this should happen in the condition table (which would also be beneficial to specify things like time derivatives etc, see Use somewhat normalized "experiments" table instead of conditions/timecourses PEtab-dev/PEtab#585).parameter values for neural networks are provided in a separate
parameters_nn.tsv
file, how about directly creating a proper parameters table where test values are set as nominal values?Code to implement a majority of the proposed changes is available here https://github.com/AMICI-dev/AMICI/blob/b201f4efc354939ac1b0763c65f2e22507d086c4/tests/sciml/testsuite.py#L177
The text was updated successfully, but these errors were encountered: