Ifpack: inefficient and inconsistent implementation of Ifpack_Hypre
Created by: ecoon
Ifpack_Hypre is inconsistent with documentation and design of Ifpack in general, and has several bugs that make it unusable with Hypre's Hypre_BoomerAMGSetDofFunc()
option.
@trilinos/Ifpack
Current Behavior
Ifpack_Hypre is inconsistent with documentation and design of Ifpack in general. Specifically, Ifpack's user guide states that (in regards to the general interface design and usage)Initialize()
is work done on a symbolically assembled matrix, and uses only matrix structure (not values), while Compute()
is work done an a fully assemble matrix, and relies on values. Then, ApplyInverse()
does exactly that. Logically (and I assume this was the intended design), it should be possible to both:
- call
ApplyInverse()
multiple times for the same values without re-callingCompute()
(this is currently true) - call
Compute()
multiple times with different values but the same sparsity structure (this is not currently true)
Currently Compute()
completely destroys the Hypre solver context, which is unnecessary/inefficient. Furthermore only Initialize()
copies values from the Epetra Matrix to the Hypre Matrix, meaning that calling Compute()
is not sufficient after values change -- instead Initialize()
must also be called again (I would call this a correctness issue, at least within the constraints of the manual).
Additionally, there are bugs related to the use of Hypre_SetDofFunc()
, which is necessary for solution block systems that are not strided (e.g. hybrid / mixed formulation discretizations). There are also several gotchas in Hypre's memory management in this use case which make it impossible to use through Ifpack currently (see https://github.com/LLNL/COGENT/blob/master/hypre-2.9.0b/src/parcsr_ls/HYPRE_parcsr_amg.c#L1140 which is still a "surprise" in the current code, even if this comment was deleted!)
Expectations
I'm nearly done re-implementing this, and will provide a pull request for the code. I would like to get some help with running whatever existing tests are available to ensure correctness of my implementation if that's possible (hopefully your CI explores the Ifpack_Hypre path?). I'm happy to provide tests which show the current code's shortcomings, but these tests are currently embedded within a larger codebase, and I would need some help adapting them for Trilinos's testing harness.
Note this is also an issue in xSDK's Ifpack2_Hypre, which is effectively a copy/paste of the current Ifpack_Hypre implementation with matrix/vector interfaces updated to the Tpetra stack. It would be fairly easy to redo both of these at the same time if anyone from IDEAS-xSDK reads this.