Trilinos issueshttps://gitlab.osti.gov/jmwille/Trilinos/-/issues2018-06-13T23:28:06Zhttps://gitlab.osti.gov/jmwille/Trilinos/-/issues/2852Ifpack: inefficient and inconsistent implementation of Ifpack_Hypre2018-06-13T23:28:06ZJames WillenbringIfpack: 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.
<!---
Replace <teamName> below with...*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.
<!---
Replace <teamName> below with the appropriate Trilinos package/team name.
-->
@trilinos/Ifpack
## Current Behavior
<!---
Tell us how the current behavior fails to meet your expectations in some way.
-->
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.https://gitlab.osti.gov/jmwille/Trilinos/-/issues/2167syntax error in Ifpack_SPARSKIT.cpp ?2018-01-17T19:14:09ZJames Willenbringsyntax error in Ifpack_SPARSKIT.cpp ?*Created by: SilverLinings89*
<!--- Provide a general summary of the issue in the Title above. -->
I tried building Trilinos with Sparskit support and got the error
> trilinos-12.4.2-Source/packages/ifpack/src/Ifpack_SPARSKIT.cpp:3...*Created by: SilverLinings89*
<!--- Provide a general summary of the issue in the Title above. -->
I tried building Trilinos with Sparskit support and got the error
> trilinos-12.4.2-Source/packages/ifpack/src/Ifpack_SPARSKIT.cpp:315:11: error: ‘endl’ was not declared in this scope
I checked the source and it is still this way in the latest version. simply putting either using namespace std or std::endl for all cases fixes this. Building seems to work now.
@trilinos/ifpack
## Current Behavior
<!---
Tell us how the current behavior fails to meet your expectations in some way.
-->
Build aborts.
## Possible Solution
<!---
Not obligatory, but suggest a fix for the bug or documentation, or suggest
ideas on how to implement the addition or change.
-->
I checked the source and it is still this way in the latest version. simply putting either using namespace std or std::endl for all cases fixes this. Building seems to work now.
## Steps to Reproduce
<!---
Provide a link to a live example, or an unambiguous set of steps to reproduce
this issue. Include code to reproduce, if relevant.
1. Do this.
1. Do that.
1. Shake fist angrily at computer.
-->
I dont actually have a lot of experience with this library and I don't know if this is an unusual case because in other cases some file would be loaded that sets the namespace, but in my case it happened.
## Your Environment
<!---
Include relevant details about your environment such that we can replicate this
issue.
-->
- Ubuntu 16.04
- Gnu
https://gitlab.osti.gov/jmwille/Trilinos/-/issues/1945several packages call exit() in the library2017-11-07T18:25:18ZJames Willenbringseveral packages call exit() in the library*Created by: nschloe*
Discussions of why calling `exit()` in production code go back as far as 2010; cf. https://software.sandia.gov/bugzilla/show_bug.cgi?id=4969. Unfortunately, many libraries still call `exit()`:
```
$ lintian * | g...*Created by: nschloe*
Discussions of why calling `exit()` in production code go back as far as 2010; cf. https://software.sandia.gov/bugzilla/show_bug.cgi?id=4969. Unfortunately, many libraries still call `exit()`:
```
$ lintian * | grep shlib-calls-exit
X: libtrilinos-zoltan12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_zoltan.so.12.12.1
X: libtrilinos-aztecoo12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_aztecoo.so.12.12.1
X: libtrilinos-muelu12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_muelu.so.12.12.1
X: libtrilinos-nox12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_noxepetra.so.12.12.1
X: libtrilinos-stokhos12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_stokhos_muelu.so.12.12.1
X: libtrilinos-galeri12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_galeri-epetra.so.12.12.1
X: libtrilinos-epetraext12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_epetraext.so.12.12.1
X: libtrilinos-shylu12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_shylu.so.12.12.1
X: libtrilinos-pamgen12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_pamgen.so.12.12.1
X: libtrilinos-ml12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_ml.so.12.12.1
X: libtrilinos-triutils12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_triutils.so.12.12.1
X: libtrilinos-ifpack12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_ifpack.so.12.12.1
X: libtrilinos-pliris12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_dpliris.so.12.12.1
X: libtrilinos-trilinoscouplings12: shlib-calls-exit usr/lib/x86_64-linux-gnu/libtrilinos_trilinoscouplings.so.12.12.1
```
Would be great to see some progress here.https://gitlab.osti.gov/jmwille/Trilinos/-/issues/1196Ifpack_SparseContainer can't be used without initializing MPI2017-03-31T19:41:28ZJames WillenbringIfpack_SparseContainer can't be used without initializing MPI*Created by: Sbte*
`Ifpack_SparseContainer` can't be used when Trilinos is compiled with MPI support but it is used without calling `MPI_Initialize`. This happens because a serial MPI communicator is instantiated in the constructor:
...*Created by: Sbte*
`Ifpack_SparseContainer` can't be used when Trilinos is compiled with MPI support but it is used without calling `MPI_Initialize`. This happens because a serial MPI communicator is instantiated in the constructor:
https://github.com/trilinos/Trilinos/blob/master/packages/ifpack/src/Ifpack_SparseContainer.h#L323
A case where one might want to use this without calling `MPI_Initialize` is when `Epetra_SerialComm` is used for all vectors and matrices. Is there any reason why `Epetra_MpiComm(MPI_COMM_SELF)` is used instead of `Epetra_SerialComm` in `Ifpack_SparseContainer`? If not I can submit a pull request that gets rid of this if you like.https://gitlab.osti.gov/jmwille/Trilinos/-/issues/1145Nightly failure for XYCE build2017-05-11T17:21:06ZJames WillenbringNightly failure for XYCE build*Created by: jwillenbring*
Ifpack fails to build for the no-C++11 XYCE build as seen [here](http://testing.sandia.gov/cdash/viewBuildError.php?buildid=2788850).
@bmpersc have you looked at this at all?
This is the last current fai...*Created by: jwillenbring*
Ifpack fails to build for the no-C++11 XYCE build as seen [here](http://testing.sandia.gov/cdash/viewBuildError.php?buildid=2788850).
@bmpersc have you looked at this at all?
This is the last current failure for the XYCE build remaining related to issue #1129 https://gitlab.osti.gov/jmwille/Trilinos/-/issues/14Ifpack: set_parameters is deprecated2016-03-03T17:45:09ZJames WillenbringIfpack: set_parameters is deprecated*Created by: nschloe*
In commit 1f62d144c19d003742d03242ec501b987bc9bb23 (Sep 2011), several Ifpack functions where marked deprecated. One of them are still in use today – in Ifpack itself!
```
/«PKGBUILDDIR»/packages/ifpack/src/Ifpack...*Created by: nschloe*
In commit 1f62d144c19d003742d03242ec501b987bc9bb23 (Sep 2011), several Ifpack functions where marked deprecated. One of them are still in use today – in Ifpack itself!
```
/«PKGBUILDDIR»/packages/ifpack/src/Ifpack_IlukGraph.cpp:123:11: warning: 'void Ifpack::set_parameters(const Teuchos::ParameterList&, Ifpack::param_struct&, bool)' is deprecated [-Wdeprecated-declarations]
Ifpack::set_parameters(parameterlist, params, cerr_warning_if_unused);
^
In file included from /«PKGBUILDDIR»/packages/ifpack/src/Ifpack_IlukGraph.cpp:49:0:
/«PKGBUILDDIR»/packages/ifpack/src/ifp_parameters.h:95:24: note: declared here
IFPACK_DEPRECATED void set_parameters(const Teuchos::ParameterList& parameterlist,
^
/«PKGBUILDDIR»/packages/ifpack/src/Ifpack_IlukGraph.cpp:123:11: warning: 'void Ifpack::set_parameters(const Teuchos::ParameterList&, Ifpack::param_struct&, bool)' is deprecated [-Wdeprecated-declarations]
Ifpack::set_parameters(parameterlist, params, cerr_warning_if_unused);
^
In file included from /«PKGBUILDDIR»/packages/ifpack/src/Ifpack_IlukGraph.cpp:49:0:
/«PKGBUILDDIR»/packages/ifpack/src/ifp_parameters.h:95:24: note: declared here
IFPACK_DEPRECATED void set_parameters(const Teuchos::ParameterList& parameterlist,
^
/«PKGBUILDDIR»/packages/ifpack/src/Ifpack_IlukGraph.cpp:123:71: warning: 'void Ifpack::set_parameters(const Teuchos::ParameterList&, Ifpack::param_struct&, bool)' is deprecated [-Wdeprecated-declarations]
Ifpack::set_parameters(parameterlist, params, cerr_warning_if_unused);
^
In file included from /«PKGBUILDDIR»/packages/ifpack/src/Ifpack_IlukGraph.cpp:49:0:
/«PKGBUILDDIR»/packages/ifpack/src/ifp_parameters.h:95:24: note: declared here
IFPACK_DEPRECATED void set_parameters(const Teuchos::ParameterList& parameterlist,
```
It's either time to remove those calls or to remove the deprecation attribute.