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

Support valid AMICI noise distributions that are invalid in PEtab #1157

Merged
merged 7 commits into from
Nov 3, 2023

Conversation

dilpath
Copy link
Member

@dilpath dilpath commented Oct 25, 2023

The PEtab importer will throw "invalid PEtab" errors if a user requests e.g. a negative binomial noise distribution. The user can skip the validation check with PetabImporter(..., validate_petab=False). However, AMICI will still complain about the invalid PEtab [1], because validate_petab isn't passed to AMICI. This PR fixes this, s.t. users can use any AMICI noise distribution [2].

[1] https://github.com/AMICI-dev/AMICI/blob/3c5e997df3655c26dde35705ef25b2a0f419fe8b/python/sdist/amici/petab_import.py#L565
[2] https://github.com/AMICI-dev/AMICI/blob/3c5e997df3655c26dde35705ef25b2a0f419fe8b/python/sdist/amici/import_utils.py#L182-L188

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2023

Codecov Report

Merging #1157 (9a0101d) into develop (160c2a8) will decrease coverage by 3.82%.
Report is 411 commits behind head on develop.
The diff coverage is 87.87%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##           develop    #1157      +/-   ##
===========================================
- Coverage    88.16%   84.35%   -3.82%     
===========================================
  Files           79      148      +69     
  Lines         5257    11681    +6424     
===========================================
+ Hits          4635     9853    +5218     
- Misses         622     1828    +1206     
Files Coverage Δ
pypesto/C.py 100.00% <100.00%> (ø)
pypesto/__init__.py 100.00% <100.00%> (ø)
pypesto/engine/__init__.py 100.00% <100.00%> (ø)
pypesto/engine/multi_process.py 93.10% <100.00%> (+0.79%) ⬆️
pypesto/engine/multi_thread.py 100.00% <100.00%> (ø)
pypesto/engine/single_core.py 100.00% <100.00%> (ø)
pypesto/engine/task.py 100.00% <100.00%> (ø)
pypesto/ensemble/__init__.py 100.00% <100.00%> (ø)
pypesto/ensemble/task.py 100.00% <100.00%> (ø)
pypesto/hierarchical/__init__.py 100.00% <100.00%> (ø)
... and 138 more

@@ -280,7 +281,7 @@ def create_model(
logger.info(
f"Compiling amici model to folder " f"{self.output_folder}."
)
self.compile_model(**kwargs)
self.compile_model(validate=self.validate_petab, **kwargs)
Copy link
Member

@dweindl dweindl Oct 25, 2023

Choose a reason for hiding this comment

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

This fails for pysb models. Pass validate only if from petab.models import MODEL_TYPE_SBML; petab_problem.model.type_id == MODEL_TYPE_SBML. Requires petab>=0.1.27

Copy link
Member

@dweindl dweindl left a comment

Choose a reason for hiding this comment

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

👍

@dilpath dilpath merged commit ddf7798 into develop Nov 3, 2023
18 checks passed
@dilpath dilpath deleted the fix_petab_importer_amici_validation branch November 3, 2023 12:45
This was referenced Nov 15, 2023
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.

None yet

4 participants