Tpetra::CrsGraph::computeLocalConstants ignores Maps to find lower/upper triangular, but CrsGraph uses Maps to find diagonal entries

[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 #581 (comment) 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 Tpetra: Deprecate & remove get*NumDiags & is{Lower,Upper}Triangular #2630

[github-actions]

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.
If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.

[github-actions]

This issue was closed due to inactivity for 395 days.