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

Generation of condition-specific SBML models #108

Merged
merged 11 commits into from
Feb 16, 2022
Merged

Conversation

dweindl
Copy link
Member

@dweindl dweindl commented Feb 11, 2022

New function petab.get_model_for_condition to generate condition-specific SBML models based on a PEtab problem.

New function `petab.get_model_for_condition` to generate condition-specific SBML models based on a PEtab problem.
@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2022

Codecov Report

Merging #108 (3c0e5ed) into develop (65fd2da) will increase coverage by 0.19%.
The diff coverage is 80.43%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #108      +/-   ##
===========================================
+ Coverage    75.90%   76.10%   +0.19%     
===========================================
  Files           27       27              
  Lines         2976     3021      +45     
  Branches       721      733      +12     
===========================================
+ Hits          2259     2299      +40     
- Misses         535      537       +2     
- Partials       182      185       +3     
Impacted Files Coverage Δ
petab/sbml.py 61.95% <80.43%> (+5.84%) ⬆️
petab/parameter_mapping.py 71.51% <0.00%> (+2.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65fd2da...3c0e5ed. Read the comment docs.

@dweindl dweindl requested a review from dilpath February 11, 2022 21:07
Copy link
Member

@dilpath dilpath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Looks like compartments in a conditions table aren't yet handled

petab/sbml.py Outdated Show resolved Hide resolved
petab/sbml.py Outdated Show resolved Hide resolved
petab/sbml.py Outdated Show resolved Hide resolved
petab/sbml.py Outdated Show resolved Hide resolved
petab/sbml.py Outdated
Comment on lines 570 to 573
if sbml_species.isSetInitialConcentration():
sbml_species.setInitialConcentration(new_value)
elif sbml_species.isSetInitialAmount():
sbml_species.setInitialAmount(new_value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else: raise? or default to concentration or amount based on hasOnlySubstanceUnits?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of those two has to be set to be valid SBML iirc. But let's raise to be on the safe side.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently fine if e.g. initial assignment is used. From SBML spec:

Missing initialAmount and initialConcentration
values implies that their values either are unknown, or to be obtained from an external source, or determined
by an initial assignment (Section 4.8 on p. 56) or other SBML construct elsewhere in the model.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks. Okay, then we need to check hasOnlySubstanceUnits indeed.

@dweindl
Copy link
Member Author

dweindl commented Feb 14, 2022

Looks like compartments in a conditions table aren't yet handled

Oops. Thanks :).

@dweindl dweindl self-assigned this Feb 14, 2022
@dweindl dweindl requested a review from dilpath February 16, 2022 12:13
Copy link
Member

@dilpath dilpath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Another source of initial values is from assignment rules and algebraic rules. From SBML spec:

If the species’ constant attribute is “ false ”, the species’ quantity value may be overridden by an InitialAssignment or changed by AssignmentRule or AlgebraicRule [...]

Doesn't make sense to override such rules just to set an initial value, but maybe such a contradiction (e.g. compartment in the condition table, but also with an assignment rule) could be identified by petablint.

petab/sbml.py Outdated Show resolved Hide resolved
Co-authored-by: Dilan Pathirana <[email protected]>
@dweindl
Copy link
Member Author

dweindl commented Feb 16, 2022

Another source of initial values is from assignment rules and algebraic rules. [...]

True. It's currently not well defined what should happen in this case (see also discussion in PEtab-dev/PEtab#513). Let's remove all rules targeting overridden entities for now.

petab/sbml.py Outdated Show resolved Hide resolved
petab/sbml.py Outdated Show resolved Hide resolved
petab/sbml.py Outdated Show resolved Hide resolved
dweindl and others added 2 commits February 16, 2022 14:05
Co-authored-by: Dilan Pathirana <[email protected]>
Co-authored-by: Dilan Pathirana <[email protected]>
@dweindl dweindl merged commit b9294e6 into develop Feb 16, 2022
@dweindl dweindl deleted the model_for_condition branch February 16, 2022 13:29
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 this pull request may close these issues.

3 participants