-
Notifications
You must be signed in to change notification settings - Fork 32
Don't eliminate parameters that are initial assignment targets (pt1) #2304
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2304 +/- ##
===========================================
- Coverage 77.81% 77.79% -0.03%
===========================================
Files 318 318
Lines 20534 20536 +2
Branches 1436 1436
===========================================
- Hits 15979 15975 -4
- Misses 4552 4558 +6
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
dweindl
added a commit
to dweindl/AMICI
that referenced
this pull request
Feb 22, 2024
Currently, parameters that are targets of initial assignments don't show up as parameters or expressions in the amici model. This is rather not what most users would expect. Therefore, treat all SBML parameters that are initial assignment targets and whose initial assignment does not evaluate to a number (for those that do, see AMICI-dev#2304) as amici expressions. Those static expressions will be handled more efficiently after AMICI-dev#2303. Related to AMICI-dev#2150.
Currently, parameters that are targets of initial assignments don't show up as parameters or expressions in the amici model. This is rather not what most users would expect. As a first step: treat all SBML parameters that are initial assignment targets and whose initial assignment evaluates to a number as amici parameters. Related to AMICI-dev#2150.
1c18690 to
4939350
Compare
dweindl
added a commit
to dweindl/AMICI
that referenced
this pull request
Feb 23, 2024
Currently, parameters that are targets of initial assignments don't show up as parameters or expressions in the amici model. This is rather not what most users would expect. Therefore, treat all SBML parameters that are initial assignment targets and whose initial assignment does not evaluate to a number (for those that do, see AMICI-dev#2304) as amici expressions. Those static expressions will be handled more efficiently after AMICI-dev#2303. Related to AMICI-dev#2150.
FFroehlich
approved these changes
Feb 26, 2024
dweindl
added a commit
that referenced
this pull request
Feb 27, 2024
…2305) Currently, parameters that are targets of initial assignments don't show up as parameters or expressions in the amici model. This is rather not what most users would expect. Therefore, treat all SBML parameters that are initial assignment targets and whose initial assignment does not evaluate to a number (for those that do, see #2304) as amici expressions. Those static expressions will be handled more efficiently after #2303. Related to #2150. See also #2304.
dweindl
added a commit
to dweindl/AMICI
that referenced
this pull request
Mar 4, 2024
During PEtab import, parameters that are targets of initial assignments have so far not been turned into constant parameters, because they didn't exist in the amici model (see AMICI-dev#2304). Now that those parameters remain in the model, they should be turned into constant parameters, unless specified otherwise.
This was referenced Mar 4, 2024
dweindl
added a commit
to dweindl/AMICI
that referenced
this pull request
Mar 6, 2024
Evaluate initial assignments to parameters to determine whether the targets are amici parameters or expressions. Related to AMICI-dev#2304.
dweindl
added a commit
that referenced
this pull request
Mar 6, 2024
Evaluate initial assignments to parameters to determine whether the targets are amici parameters or expressions. Related to #2304. For example, it the initial assignment is `log(10)`, this should still be a parameter instead of an expression, but `sympy.log(10).is_Number` will be `False`.
dweindl
added a commit
to dweindl/AMICI
that referenced
this pull request
Mar 6, 2024
During PEtab import, parameters that are targets of initial assignments have so far not been turned into constant parameters, because they didn't exist in the amici model (see AMICI-dev#2304). Now that those parameters remain in the model, they should be turned into constant parameters, unless specified otherwise.
dweindl
added a commit
that referenced
this pull request
Mar 6, 2024
…2345) During PEtab import, parameters that are targets of initial assignments have so far not been turned into constant parameters, because they didn't exist in the amici model (see #2304). Now that those parameters remain in the model, they should be turned into constant parameters, unless specified otherwise. Requires PEtab-dev/libpetab-python#248, otherwise PEtab parameter mapping will provide potentially incorrect values for those parameters.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, parameters that are targets of initial assignments don't show up as parameters or expressions in the amici model. This is rather not what most users would expect.
For example, previously:
SBML parameter with
value=2-> will result in an amici parameter with that nameSBML parameter with initial assignment
2-> will not result in an amici parameter with that nameAs a first step: treat all SBML parameters that are initial assignment targets and whose initial assignment evaluates to a number as amici parameters.
Related to #2150.