Refactor NewtonSolver #2521
Refactor NewtonSolver #2521
Conversation
dweindl
force-pushed
the
fix_1164_newton_linsolwrapper
branch
from
9a79289 to
f7bd3f8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2521 +/- ##
===========================================
+ Coverage 77.67% 77.75% +0.07%
===========================================
Files 324 323 -1
Lines 20951 20915 -36
Branches 1451 1446 -5
===========================================
- Hits 16274 16262 -12
+ Misses 4674 4650 -24
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
dweindl
force-pushed
the
fix_1164_newton_linsolwrapper
branch
2 times, most recently
from
ecc025e to
51ff7ce
Compare
src/newton_solver.cpp
Outdated
| void NewtonSolverDense::reinitialize() { | ||
| /* dense solver does not need reinitialization */ | ||
| void NewtonSolver::reinitialize() { | ||
| // dense solver does not need reinitialization |
There was a problem hiding this comment.
| // dense solver does not need reinitialization | |
| // direct solvers does not need reinitialization |
what about iterative solvers?
There was a problem hiding this comment.
They are already filtered out in the constructor:
Lines 19 to 30 in 12ee3eb
Some methods will have to be updated when more solvers are to be supported.
There was a problem hiding this comment.
Added comment+exception regarding the need for updating those two functions to support additional solvers.
| if (auto s = dynamic_cast<SUNLinSolKLU const*>(linsol_.get())) { | ||
| return s->is_singular(); | ||
| } | ||
|
|
||
| // dense solver doesn't have any implementation for rcond/condest, so use |
There was a problem hiding this comment.
what about other direct/implicit solvers?
There was a problem hiding this comment.
As above -- they haven't been supported before this PR, and they are still not supported yet. Although it would be pretty easy to extend that now....
dweindl
force-pushed
the
fix_1164_newton_linsolwrapper
branch
3 times, most recently
from
cda7aac to
00602c8
Compare
Use SUNLinSolWrapper. Get rid of subclasses. Simplify. Closes AMICI-dev#1164. We should also be able to get rid of the remaining data members, but that's for another time...
dweindl
force-pushed
the
fix_1164_newton_linsolwrapper
branch
from
00602c8 to
39dfa38
Compare
Use SUNLinSolWrapper. Get rid of subclasses. Simplify.
Closes #1164.
We should also be able to get rid of the remaining data members,
but that's for another time...