Fix 1422 #1461
Fix 1422 #1461
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1461 +/- ##
===========================================
+ Coverage 79.15% 79.19% +0.03%
===========================================
Files 66 66
Lines 10191 10196 +5
===========================================
+ Hits 8067 8075 +8
+ Misses 2124 2121 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
src/steadystateproblem.cpp
Outdated
| throw AmiException("Steady state sensitivity computation failed due " | ||
| "to unsuccessful factorization of RHS Jacobian"); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Is this really necessary? To me it looks like the sensi are already initialized above (and that code also accounts for whether we are doing pre/post-equilibration).
There was a problem hiding this comment.
I just tried in my first commit what happens if we just skip the computeNewtonSensis().
There was an error for the Raimundez model, i.e. a model with preequilibration. In that case, no solver object had been created at that point and it ran into a core dump
terminate called after throwing an instance of 'std::out_of_range'
what(): vector::_M_range_check: __n (which is 0) >= this->size() (which is 0)
Aborted (core dumped)
So I guess the problem was a sx0 wasn't written to sx_. This is, why I added this...
I'm happy about any better idea
There was a problem hiding this comment.
I mean the first code block will be executed, no matter what you do in the changes.
In your first code attempt, you modified the flag whether sensitivities are necessary or not. What I am proposing here is:
if (getSensitivityFlag(model, solver, it, SteadyStateContext::newtonSensi) && numsteps_[0] > 0)
{
try {
/* this might still fail, if the Jacobian is singular and
simulation did not find a steady state */
newtonSolver->computeNewtonSensis(sx_);
} catch (NewtonFailure const &) {
/* No steady state could be inferred. Store simulation state */
storeSimulationState(model, solver->getSensitivityOrder() >=
SensitivityOrder::first);
throw AmiException("Steady state sensitivity computation failed due "
"to unsuccessful factorization of RHS Jacobian");
}
}
There was a problem hiding this comment.
I think the failures in the tests are due to reinitialization with the initial state instead of keeping the simulated state.
There was a problem hiding this comment.
You're right, the initialization has been done anyway... I think what you propose is logically equivalent to changing the flag, but let's give it a try...
There was a problem hiding this comment.
You're right, the initialization has been done anyway... I think what you propose is logically equivalent to changing the flag, but let's give it a try...
storeSimulationState(model, getSensitivityFlag(model, solver, it, SteadyStateContext::sensiStorage));
There was a problem hiding this comment.
Thanks for pointing that one out!
The whole logic when which sensitivity is need in that code is frustrating sometimes...
|
Kudos, SonarCloud Quality Gate passed! |
fixes #1422