Fix BaseThinLayer removing species occurring in equations
#90
+259
−23
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
Additional assertions verify that data_unit is not None for species_data[0] and species_data[1] in TestTabularImport, improving test coverage for unit presence.
Updates BaseThinLayer to retain species that are referenced in ODE, assignment, initial assignment, and kinetic law equations, even if they are not part of reactions or defined as ODEs. Adds tests to verify that such species are not removed during optimization.
Member
|
Still does not work, see discussion in #86. |
Improves species value assignment in InitMap by checking for both '<species>_init' and '<species>' attributes in the model, raising an error if neither is found. Also updates import order and minor formatting for consistency.
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.
As mentioned in #86, the current implementation disregards species that are not explicitly included in either a reaction scheme or an ODE but are part of either a kinetic law or an ODE equation. For instance, as illustrated in the PGM-ENO dataset, the protein concentration
prot01is not explicitly modeled in a reaction scheme but is part of the kinetic law. Therefore, it should not be removed because it is an integral part of the system. Discarding it results in undefined behavior, as documented in #86.This pull request enhances the logic for filtering species in EnzymeML documents to ensure that all species referenced in equations, whether in ODEs, assignments, initial assignments, or kinetic laws, are retained, even if they are not directly involved in reactions. Additionally, new and more comprehensive tests have been added to validate this behavior. Minor improvements to unit tests for CSV import are also included.
Filtering logic improvements:
_remove_unmodeled_speciesmethod inbase.pyto collect all species referenced in any equation (ODE, assignment, initial assignment, kinetic law) and ensure these are not removed from the document, using symbolic analysis viasympy. Also added a helper method_get_all_speciesto retrieve all species IDs. [1] [2] [3] [4]Testing enhancements:
test_thinlayer.pythat verify species referenced in equations (but not in reactions or ODEs) are correctly retained after filtering. These cover ODE, assignment, initial assignment, and kinetic law scenarios, and use a new species exclusively referenced in equations.test_thinlayer.pyfor completeness.CSV import test improvements:
test_tabular.pyto ensure that data units are present and correct for imported species measurements. [1] [2]Closes issues
ThinLayerPysces: proteins not updated during simulation #86This change is