GitLab

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:

1. call ApplyInverse() multiple times for the same values without re-calling Compute() (this is currently true)
2. 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.