Enable changing the residual tolerance after a batch solver is created #1087
Conversation
| * | ||
| * @return The residual tolerance. | ||
| */ | ||
| double get_residual_tolerance() const { return residual_tol_; } |
There was a problem hiding this comment.
I think we should maintain just one copy of the tolerance instead of two separate copies. If the users sets a tolerance, then we just modify the existing one instead of adding one more parameter. the parameters_ struct already has a getter as well
There was a problem hiding this comment.
Alternatively: Get rid of the parameters member entirely. It's hidden in one of the factory macros, but TBH I dislike how well they hide the additional member they add. I worked entirely without the macros in #1082, and it felt much cleaner :)
There was a problem hiding this comment.
There are two types of parameters: Ones that describe how to generate an object, but can be discarded afterwards, and ones that describe the runtime behavior of the object. IMO, we should only keep the latter inside the generated object.
There was a problem hiding this comment.
@upsj That has the advantage that I could add the residual tolerance, getter and setter only to the base class. @pratikvn The problem with the factory macros is that the entire struct needs to be in the derived class, so we'd need to duplicate the getter and setter on every batched iterative solver, which should ideally be unnecessary. That would also impose the necessity to know the derived type just to set/get the residual tolerance.
There was a problem hiding this comment.
But I think having two things settable in two different ways might bring a lot more confusion, so we should unify those.
deadcfb to
0cac499
Compare
|
format! |
16fa87b to
80bc6f8
Compare
31e5be6 to
aa54714
Compare
d0ffd73 to
0977810
Compare
|
Note: This PR changes the Ginkgo ABI: For details check the full ABI diff under Artifacts here |
|
Kudos, SonarCloud Quality Gate passed!
|
This adds an interface to change the residual tolerance for subsequent runs of a solver, so that a new factory does not have to be created just for changing the tolerance. In cases where batch scaling is needed, this removes the need for redoing the setup task of scaling just for changing the tolerance, among potentially other things in the future.
A new, non-templated base class
BatchSolveris added. This contains all functionality that is agnostic of the concrete type - preconditioner, generated preconditioner, scaling operators, residual tolerance, maximum iteration count. Thus, the user only needs a pointer toBatchLinOpfor applying the solver, and a dynamic cast toBatchSolverto get and set/get solver-agnostic parameters like the residual tolerance.The factory parameter is now renamed to
default_residual_tol, and this is used to initialize theresidual_tol_values of generated solvers. Theset_residual_tolerance(double)member funct 10000 ion directly sets theresidual_tol_on the solver object.Drawback:
real_types of the solvers. This was done so that the residual tolerance can be updated independent of the concrete batched solver type - the application does not always need to know the concrete type.