Tpetra::CrsGraph::computeLocalConstants ignores Maps to find lower/upper triangular, but CrsGraph uses Maps to find diagonal entries
Created by: mhoemmen
Tpetra::CrsGraph::computeLocalConstants
only looks at the local index values to find whether the graph is structurally locally lower or upper triangular. However, computeLocalConstants
and other CrsGraph methods (e.g., getLocalDiagOffsets
) use global indices to identify diagonal entries.
This means that users who expect lower / upper triangular status to rely on global indices, may get wrong answers when they call Tpetra::CrsMatrix::localSolve
. Furthermore, attempts to fix this may cause surprising test failures in application code.
Ifpack2's incomplete factorizations and Ifpack2::AdditiveSchwarz
work around this (intentionally or not) by using Ifpack2::LocalFilter
. This class assigns new, contiguous row and column Maps. Contiguous Maps don't have this problem. (This is a long-known issue. See Bug 5992 in the old Bugzilla system, circa November 2013.) However, Ifpack2 preconditioners that do not use LocalFilter may get the wrong answer, especially if they identify and pre-extract the diagonal entries (as e.g., Ifpack2::Relaxation
does). It's not clear whether this could break Ifpack2, but it may be worth investigating.
This issue is coming to the fore once again, because of the urgency of #2630. Fixing #2630 will force detection of local lower/upper triangularity into downstream solvers, like Ifpack2::LocalSparseTriangularSolver
. Comments like https://github.com/trilinos/Trilinos/issues/581#issuecomment-384526949 suggest that HTS, an optimized sparse triangular solver in Trilinos, only uses local indices to determine the triangular structure. It may make sense for now, for Ifpack2 to use only local indices, and for LocalSparseTriangularSolver to reject noncontiguous Maps. I encourage discussion on this issue.
@trilinos/tpetra @trilinos/ifpack2 @ambrad @aprokop
Expectations
The way that Tpetra and/or Ifpack2 identify diagonal entries, should be consistent with the way that Tpetra and/or Ifpack2 identify local lower/upper triangular structure.
Current Behavior
Tpetra::CrsGraph::computeLocalConstants uses only local indices to identify local lower/upper triangular structure, but CrsGraph uses global indices (the row and column Maps together) to identify diagonal entries.
Motivation and Context
Fixing #2630 means purging local lower/upper triangular structure detection from Tpetra. Ifpack2::LocalSparseTriangularSolver
needs this detection for local sparse triangular solves. LocalSparseTriangularSolver could just make users tell it about the local triangular structure; this would work for Ifpack2 classes like ILUT and RILUK, but would not work for actual direct users of LocalSparseTriangularSolver.
Related Issues
- Related to #2630