Add support for DAE models#555
Open
agarny wants to merge 6 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses #544 by extending libOpenCOR’s SED-ML ingestion so DAE models can be instantiated/run when an NLA solver is provided via legacy OpenCOR SED-ML annotations (and it also introduces parsing of a libOpenCOR nlaAlgorithm element). It adds a comprehensive set of DAE SED-ML fixtures and unit/coverage tests to validate expected solver selection and warning behavior.
Changes:
- Add SED-ML parsing for NLA solver settings from (1) libOpenCOR
nlaAlgorithmelements and (2) legacy OpenCOR<annotation><nlaSolver …/></annotation>blocks. - Add DAE SED-ML/OMEX test fixtures covering valid, missing, duplicate, and unknown NLA solver configurations.
- Add new instance and coverage tests for DAE model loading/execution and warning paths; bump version and tweak issue printing formatting.
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| VERSION.txt | Version bump. |
| src/misc/utils.cpp | Print issues header now includes a newline. |
| src/support/sedml/sedmlfile.cpp | Parse NLA solver from libOpenCOR nlaAlgorithm and legacy annotation; integrate into simulation population. |
| src/support/sedml/sedmlfile_p.h | Add NlaSolverInfo and store original SED-ML file contents for XML parsing. |
| tests/api/sed/instancetests.cpp | Add instance tests for running DAE models from CellML/SED-ML/OMEX (legacy and non-legacy). |
| tests/api/sed/coveragetests.cpp | Add coverage tests for warnings/branches around NLA solver discovery (new + legacy paths). |
| tests/res/api/sed/dae/model.sedml | DAE SED-ML fixture using libOpenCOR nlaAlgorithm. |
| tests/res/api/sed/dae/model_unknown_nla_algorithm.sedml | Fixture with unknown nlaAlgorithm kisaoID to trigger fallback warning. |
| tests/res/api/sed/dae/model_several_nla_algorithms.sedml | Fixture with multiple nlaAlgorithm elements to trigger “first used” warning. |
| tests/res/api/sed/dae/model_no_nla_algorithm.sedml | Fixture missing kisaoID on nlaAlgorithm to trigger fallback warning. |
| tests/res/api/sed/dae/model_nla_algorithm_and_nla_algorithm.sedml | Fixture where NLA solver is set both via algorithm and nlaAlgorithm to trigger “ignored” warning. |
| tests/res/api/sed/dae/model_legacy.sedml | Legacy annotation-based NLA solver fixture (OpenCOR namespace). |
| tests/res/api/sed/dae/model_legacy_unknown_property.sedml | Legacy fixture with an unknown solver property to trigger “ignored property” warning. |
| tests/res/api/sed/dae/model_legacy_unknown_property_namespace.sedml | Legacy fixture with a property in an unexpected namespace (coverage path). |
| tests/res/api/sed/dae/model_legacy_unknown_property_element.sedml | Legacy fixture with an unexpected property element name (coverage path). |
| tests/res/api/sed/dae/model_legacy_unknown_nla_solver.sedml | Legacy fixture with unknown nlaSolver name to trigger fallback warning. |
| tests/res/api/sed/dae/model_legacy_unknown_namespace.sedml | Legacy fixture with unknown annotation namespace (coverage path). |
| tests/res/api/sed/dae/model_legacy_unknown_element.sedml | Legacy fixture with unknown annotation element name (coverage path). |
| tests/res/api/sed/dae/model_legacy_several_nla_solvers.sedml | Legacy fixture with multiple nlaSolver nodes to trigger “first used” warning. |
| tests/res/api/sed/dae/model_legacy_nla_algorithm_and_nla_solver.sedml | Legacy fixture specifying NLA solver both via algorithm and annotation to trigger precedence warning. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Check for `"0 leaks for 0 total leaked bytes."` anywhere in the output upfront (handles both old and new `leaks` output formats). - Add an `if lines:` guard so an empty `lines` string is never appended as a false positive error.
Not sure why, but it's not working on Ubuntu anymore?!
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
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.
Fixes #544.