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

Set known defaults for a CellModels ODESystem #100

Merged
merged 2 commits into from
Dec 16, 2022

Conversation

bauglir
Copy link
Contributor

@bauglir bauglir commented Dec 16, 2022

CellML files contain initial_value fields specifying initial values for parameters and states in the system. Retrieving these for the ODESystem was already possible, but had to be done separately or as part of the ODEProblem constructor defined for CellModels. This change makes sure they're added as defaults of the ODESystem instead simplifying code that's consuming these ODESystems.

Defaults are only set for simplified ODESystems. Unsimplified ODESystems may contain parameters and states for which no defaults are available resulting in errors trying to construct the ODESystem. These errors will still occur when using the result of calling process_components with simplify = false, but as noted in the code that should only be done for debugging purposes and this functionality is not exposed at the higher API levels.

Fixes #56.

CellML files contain `initial_value` fields specifying initial values
for parameters and states in the system. Retrieving these for the
`ODESystem` was already possible, but had to be done separately or as
part of the `ODEProblem` constructor defined for `CellModel`s.

Defaults are only set for simplified `ODESystem`s. Unsimplified
`ODESystem`s may contain parameters and states for which no defaults are
available resulting in errors trying to construct the `ODESystem`. These
errors will still occur when using the result of calling
`process_components` with `simplify = false`, but as noted in the code
that should only be done for debugging purposes and this functionality
is not exposed at the higher API levels.

Fixes SciML#56.
@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Merging #100 (3ecae1c) into master (a879de9) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
+ Coverage   83.66%   83.70%   +0.04%     
==========================================
  Files           6        6              
  Lines         355      356       +1     
==========================================
+ Hits          297      298       +1     
  Misses         58       58              
Impacted Files Coverage Δ
src/components.jl 93.33% <100.00%> (+0.04%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@anandijain
Copy link
Contributor

awesome, thanks joris

@anandijain anandijain merged commit ebee603 into SciML:master Dec 16, 2022
@bauglir bauglir deleted the set-ODESystem-defaults branch December 16, 2022 23:51
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.

At ODESystem/CellModel construction time, use list_states and list_params as defaults in constructor
2 participants