-
Notifications
You must be signed in to change notification settings - Fork 90
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
Remove legacy Solver
API
#2285
Remove legacy Solver
API
#2285
Conversation
Always store the Jacobian if set, but defer to PhysicsModel Jacobian if that's set
If preconditioner or Jacobian was set using `PhysicsModel::setPrecon/setJacobian` then CVODE, ARKODE and PETSc `Solver`s would not use them
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.
clang-tidy made some suggestions
status = model->runConvective(t); | ||
}else | ||
status = (*phys_conv)(t); | ||
status = model->runConvective(t); |
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.
warning: Value stored to status
is never read [clang-analyzer-deadcode.DeadStores]
status = model->runConvective(t);
^
src/solver/solver.cxx:1239:5: note: Value stored to 'status' is never read
I've never used the |
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.
bout-v5-physics-model-upgrader.py
was too much for me to get my head around, so I skipped that, but everything else looks good to me.
Grepping the models in examples, it looks like the
which is not terribly many. I think I will them for now, maybe we can have a bit of a discussion about the direction to go in at the BUG meeting. |
I agree this is a good idea, and having one way to do things is definitely a good idea. I would like to think about what changes we might like to make in future, so that we don't have to change interface again soon. |
I think either interface, via Picking an interface can wait for another PR when we've thought about it a bit more. |
Remove the following Solver methods:
Solver::setRHS
Solver::setPrecon
Solver::setJacobian
Solver::setSplitOperator
and the following free functions:
bout_solve
bout_run
bout_constrain
physics_run
physics_init
bin/bout-v5-physics-model-upgrader.py
can now fix all of the above.Do we also want to remove
Physics::bout_solve/constrain
? Currently this just callsSolver::add/constrain
. One could imagine removing or hiding thePhysicsModel::solver
member, which would make keeping thebout_solve/constrain
methods appealing.(includes the bug fix in #2283)