-
Notifications
You must be signed in to change notification settings - Fork 31
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 import: fix handling of fixed parameters for rule targets #1915
Conversation
Previously, targets of initial assignments or rules could have been selected as fixed parameters, which would lead to errors during model import. Those target parameters should be ignored.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #1915 +/- ##
===========================================
- Coverage 76.01% 75.99% -0.02%
===========================================
Files 76 76
Lines 12957 12960 +3
===========================================
Hits 9849 9849
- Misses 3108 3111 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Hmm, but this would be an invalid petab file then, right? |
No, I don't mean that the targets were specified as non-estimated in the parameters table, but that during amici import most non-estimated parameters are turned into fixed parameters for performance reasons. And the current logic tries to also do that for rule-targets, which is wrong. (Problem introduced in #1810) |
Ah, I see, so the parameter table doesn't need to account for all parameters in a model and the rest are assumed to remain at their default values, right? |
Kudos, SonarCloud Quality Gate passed! |
Correct. |
Previously, targets of initial assignments or rules could have been selected as fixed parameters, which would lead to errors during model import. Those target parameters should be ignored.