-
Notifications
You must be signed in to change notification settings - Fork 31
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
fix equilibraton status discrepancy #1991
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Co-authored-by: Daniel Weindl <[email protected]>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #1991 +/- ##
===========================================
- Coverage 76.06% 76.02% -0.05%
===========================================
Files 76 76
Lines 13002 13003 +1
===========================================
- Hits 9890 9885 -5
- Misses 3112 3118 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
forward sensitivities ODEs is coupled. If 'integrationOnly' approach is | ||
chosen for sensitivity computation it is enforced that steady state is | ||
computed only by numerical integration as well. */ | ||
bool turnOffNewton = solver.getNewtonMaxSteps() == 0 || ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need ((it == -1 && solver.getSensitivityMethodPreequilibration() == SensitivityMethod::forward) || solver.getSensitivityMethod() == SensitivityMethod::forward))
anyways?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latter is necessary as numerical integration of the model ODEs and corresponding forward sensitivities ODEs is coupled. If 'integrationOnly' approach is chosen for sensitivity computation it is enforced that steady state is computed only be numerical integration as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns off Newton's method if SteadyStateSensitivityMode::integrationOnly is set together with SensitivityMethod::forward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, makes sense, thanks :)
Kudos, SonarCloud Quality Gate passed! |
Turn off Newton's method if newton_maxsteps is set to 0, so that in that case the resulting status will be
[0 1 0]
([Did not run; Successful run; Did not run]) if numerical integration was successful. Before the status was[-2 1 0]
([Error: The method did not converge to a steady state within the maximum number of steps (Newton's method or simulation). ; Successful run; Did not run]).Overall Newton's method is turned off if newton_maxsteps is set to 0 or if 'integrationOnly' approach is chosen for sensitivity computation in combination with forward sensitivities approach. The latter is necessary as numerical integration of the model ODEs and corresponding forward sensitivities ODEs is coupled. If 'integrationOnly' approach is chosen for sensitivity computation it is enforced that steady state is computed only be numerical integration as well.