Added new struct to hold convergence parameters for the Krylov solvers. #1172
Conversation
oseikuffuor1
requested review from
liruipeng,
ulrikeyang,
rfalgout,
waynemitchell and
victorapm
|
Hi Daniel, the idea seems good. I have nothing to add. Thanks! |
|
Hi Daniel. Can you explain why this helps in the mixed precision setting? I think a use case would be good to have. Otherwise, I've got a few suggestions and questions that I'll make separately. Thanks! |
|
|
||
| HYPRE_Real rel_residual_norm; | ||
|
|
||
| } hypre_conv_params; |
There was a problem hiding this comment.
This doesn't conform to our naming conventions. Should be hypre_ConvParams.
There was a problem hiding this comment.
@rfalgout yes, I agree. I expected to change this at some point.
| HYPRE_Real cf_tol; | ||
| HYPRE_Real a_tol; | ||
| HYPRE_Real rtol; | ||
| void *conv_params; |
There was a problem hiding this comment.
Why does this have to be void *? I think we should be able to make this opaque to this header file while still retaining some type checking by doing something similar to hypre_SStructGrid_struct in sstruct_mv/HYPRE_sstruct_mv.h for example. In that header file, HYPRE_SStructGrid is used in many of the function prototypes, but it's definition is opaque in that header file. We give it a specific definition in the C source files by way of the lower-case header file. It looks like you need to know the type of the void * once you get into the pcg.c file.
There was a problem hiding this comment.
@rfalgout indeed I can do something similar for this. The type of the struct will be inferred by the precision in which we are building functions in pcg.c. So the struct's precision type will be the same as the function's.
@rfalgout in the mixed-precision setting, we would essentially have the pcg_data struct a member variable hypre_ConvParams that is specific to the precision in which the pcg_data object was created. So any calls to functions that access variables in this struct will use a consistent precision type associated with the calling function. In other words, it would be like working with a different pcg_data object for float, double or longdouble function calls (without explicitly changing the object type to pcg_data_<flt,dbl,ldbl>). For example, the function hypre_PCGGetFinalRelativeResidualNorm( void *pcg_vdata, HYPRE_Real *relative_residual_norm ) in pcg.c returns a HYPRE_Real rel_residual_norm data member of the pcg_data struct. Suppose one calls hypre_PCGGetFinalRelativeResidualNorm_flt(pcg_solver, &norm); this should return the rel_residual_norm in float. However, at runtime, HYPRE_Real could be double precision (by default if we do not use --enable-<single, longdouble>), which will then be returning a double precision value into a single precision variable. Now, if we have the hypre_ConvParams struct, the call to hypre_PCGGetFinalRelativeResidualNorm_flt would return the rel_residual_norm data member from hypre_ConvParams_flt (we will change the struct name like we do for the function names in the object file to match the precision for which they were created), which will then be consistent with precision of the callig function. |
This PR adds a new struct in the krylov directory to hold convergence parameters for the krylov solvers. This changes how we access these parameters. We will now access the tolerances and residual norm via this new struct member of the Krylov solver data, rather than directly through the solver data. This makes it convenient to define different precisions of these parameters without changing the solver data, which is favorable to mixed-precision algorithm development.