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

More flexible state reinitialization / PEtab import reinitialization fixes #1417

Merged
merged 25 commits into from
Feb 18, 2021

Conversation

dweindl
Copy link
Member

@dweindl dweindl commented Feb 12, 2021

Adds possibility to provide state indices for selective reinitialization based on fixed parameters.

The previous ExpData::reinitializeFixedParameterInitialStates is still there, but will be removed in an upcoming version.

Addresses #1345, #1396, #1319

Supersedes #1344

Include new test cases from PEtab-dev/petab_test_suite#35

@dweindl dweindl changed the base branch from master to develop February 12, 2021 21:55
@codecov
Copy link

codecov bot commented Feb 12, 2021

Codecov Report

Merging #1417 (21b38ce) into develop (7621bec) will decrease coverage by 0.04%.
The diff coverage is 82.35%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1417      +/-   ##
===========================================
- Coverage    79.07%   79.03%   -0.05%     
===========================================
  Files           66       66              
  Lines        10061    10099      +38     
===========================================
+ Hits          7956     7982      +26     
- Misses        2105     2117      +12     
Flag Coverage Δ
cpp 75.71% <78.04%> (-0.09%) ⬇️
petab 69.59% <100.00%> (+0.11%) ⬆️
python 67.61% <10.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/amici/abstract_model.h 0.00% <ø> (ø)
include/amici/edata.h 0.00% <ø> (ø)
include/amici/model.h 66.66% <ø> (ø)
include/amici/simulation_parameters.h 66.66% <ø> (ø)
src/abstract_model.cpp 3.09% <ø> (ø)
src/simulation_parameters.cpp 71.42% <33.33%> (-28.58%) ⬇️
src/edata.cpp 88.01% <86.66%> (-0.27%) ⬇️
src/model.cpp 84.33% <94.11%> (+0.09%) ⬆️
python/amici/ode_export.py 92.77% <100.00%> (ø)
python/amici/petab_import.py 71.74% <100.00%> (+0.25%) ⬆️
... and 4 more

@@ -409,6 +416,9 @@ void ConditionContext::applyCondition(const ExpData *edata,
"not match ExpData (%zd).",
model_->nk(), edata->fixedParameters.size());
model_->setFixedParameters(edata->fixedParameters);
if(!edata->reinitializeFixedParameterInitialStates)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused, don't we want to check for edata->reinitializeFixedParameterInitialStates here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is, that if edata->reinitializeFixedParameterInitialStates == true, then this has been set on Model a couple of lines before. This will populate the index array in Model. If we would execute the next line, this would overwrite them with an empty array.
Not great, but keeps backward compatibility for the moment and still allow selective reinitialization. To be cleaned up soon.

@sonarcloud
Copy link

sonarcloud bot commented Feb 18, 2021

@dweindl dweindl merged commit aa59766 into develop Feb 18, 2021
@dweindl dweindl deleted the petab_reinit branch February 18, 2021 09:56
@dweindl dweindl restored the petab_reinit branch February 18, 2021 09:56
@dweindl dweindl linked an issue Feb 19, 2021 that may be closed by this pull request
@dweindl dweindl mentioned this pull request Feb 19, 2021
dweindl added a commit that referenced this pull request Feb 20, 2021
….13)

Breaking changes:
* AMICI requires Python>=3.7
* Updated package installation (PEP517/518): 
  Creating source distributions requires https://github.com/pypa/build (#1384)
  (but now handles all package building dependencies properly)

Features:
* More flexible state reinitialization (#1417)

Updated dependencies:
* Upgrade to sundials 5.7.0 (#1392)

Fixes:
* Python: account for heaviside functions in expressions (#1382)
* Python: allow loading of existing models in import_petab_problem (#1383)
* Python: Don't override user-provided compiler/linker flags (#1389)
* Python: PEtab import reinitialization fixes (#1417)
* Python: Fix PEtab observables for pysb models (#1390)
* Python: Substitute expressions in event condition expressions (#1404)
* Python: Unspecified initial states in PEtab conditions table default to SBML initial value (#1397)
* C++: Fix timepoint out of bounds access (#1402)
* C++: Fix exported CMake config (#1388)
* Fixed Dockerfile: add python3-venv (#1398, #1408)

Other:
* Slim exported swig interface (#1425)
* Updated documentation
    * Getting started tutorial (#1423)
    * List supported SBML test tags (#1428)
    * Add AMICI C++/Python/Matlab feature comparison (#1409)
    * ...
* Various minor CI improvements
* ...
@dweindl dweindl deleted the petab_reinit branch February 20, 2021 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants