Revert unintended changes

trilinos · Sep 1, 2023 · 3d15ed1 · 3d15ed1
1 parent 1886fac
commit 3d15ed1
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 23 deletions.
diff --git a/packages/ifpack2/src/Ifpack2_Details_FastILU_Base_decl.hpp b/packages/ifpack2/src/Ifpack2_Details_FastILU_Base_decl.hpp
@@ -131,6 +131,8 @@ template<class Scalar, class LocalOrdinal, class GlobalOrdinal, class Node>
      */
     void setParameters (const Teuchos::ParameterList& List);
 
+    bool isBlockCrs() const;
+
     //! Initialize the preconditioner
     void initialize();
 

diff --git a/packages/ifpack2/src/Ifpack2_Details_FastILU_Base_def.hpp b/packages/ifpack2/src/Ifpack2_Details_FastILU_Base_def.hpp
@@ -156,6 +156,13 @@ setParameters (const Teuchos::ParameterList& List)
   params_ = Params(List, getName());
 }
 
+template<class Scalar, class LocalOrdinal, class GlobalOrdinal, class Node>
+bool FastILU_Base<Scalar, LocalOrdinal, GlobalOrdinal, Node>::
+isBlockCrs() const
+{
+  return params_.blockCrs && params_.blockCrsSize > 1;
+}
+
 template<class Scalar, class LocalOrdinal, class GlobalOrdinal, class Node>
 void FastILU_Base<Scalar, LocalOrdinal, GlobalOrdinal, Node>::
 initialize()
@@ -172,30 +179,25 @@ initialize()
     throw std::runtime_error(std::string("Called ") + getName() + "::initialize() but matrix was null (call setMatrix() with a non-null matrix first)");
   }
 
-  if (params_.blockCrs) {
+  if (isBlockCrs()) {
     auto crs_matrix = Ifpack2::Details::getCrsMatrix(this->mat_);
-    CrsArrayReader<Scalar, ImplScalar, LocalOrdinal, GlobalOrdinal, Node>::getStructure(mat_.get(), localRowPtrsHost_, localRowPtrs_, localColInds_);
-    CrsArrayReader<Scalar, ImplScalar, LocalOrdinal, GlobalOrdinal, Node>::getValues(mat_.get(), localValues_, localRowPtrsHost_);
-
-    // Create new TCrsMatrix with the new filled data
-    if (params_.blockCrsSize > 1) {
-      if (params_.fillBlocks) {
-        auto crs_matrix_block_filled = Tpetra::fillLogicalBlocks(*crs_matrix, params_.blockCrsSize);
-        auto bcrs_matrix = Tpetra::convertToBlockCrsMatrix(*crs_matrix_block_filled, params_.blockCrsSize);
-        mat_ = bcrs_matrix;
-      }
-      else {
-        auto bcrs_matrix = Tpetra::convertToBlockCrsMatrix(*crs_matrix, params_.blockCrsSize);
-        mat_ = bcrs_matrix;
-      }
 
-      CrsArrayReader<Scalar, ImplScalar, LocalOrdinal, GlobalOrdinal, Node>::getStructure(mat_.get(), localRowPtrsHost_, localRowPtrs_, localColInds_);
-      CrsArrayReader<Scalar, ImplScalar, LocalOrdinal, GlobalOrdinal, Node>::getValues(mat_.get(), localValues_, localRowPtrsHost_);
+    if (params_.fillBlocks) {
+      // Create new TCrsMatrix with the new filled data and conver to Bcrs
+      auto crs_matrix_block_filled = Tpetra::fillLogicalBlocks(*crs_matrix, params_.blockCrsSize);
+      auto bcrs_matrix = Tpetra::convertToBlockCrsMatrix(*crs_matrix_block_filled, params_.blockCrsSize);
+      mat_ = bcrs_matrix;
+    }
+    else {
+      // Assume input is already filled, just convert to Bcrs
+      auto bcrs_matrix = Tpetra::convertToBlockCrsMatrix(*crs_matrix, params_.blockCrsSize);
+      mat_ = bcrs_matrix;
     }
   }
 
   Kokkos::Timer copyTimer;
   CrsArrayReader<Scalar, ImplScalar, LocalOrdinal, GlobalOrdinal, Node>::getStructure(mat_.get(), localRowPtrsHost_, localRowPtrs_, localColInds_);
+  CrsArrayReader<Scalar, ImplScalar, LocalOrdinal, GlobalOrdinal, Node>::getValues(mat_.get(), localValues_, localRowPtrsHost_);
   crsCopyTime_ = copyTimer.seconds();
 
   if (params_.use_metis)

diff --git a/packages/ifpack2/src/Ifpack2_Details_Filu_def.hpp b/packages/ifpack2/src/Ifpack2_Details_Filu_def.hpp
@@ -110,10 +110,10 @@ initLocalPrec()
 
   bool skipSortMatrix = !matCrs.is_null() && matCrs->getCrsGraph()->isSorted() &&
                         !p.use_metis;
-  const int blockCrsSize = p.blockCrs ? p.blockCrsSize : 1;
-  localPrec_ = Teuchos::rcp(new LocalFILUB(skipSortMatrix, this->localRowPtrs_, this->localColInds_, this->localValues_, nRows, p.sptrsv_algo,
-                                          p.nFact, p.nTrisol, p.level, p.omega, p.shift, p.guessFlag ? 1 : 0, p.blockSizeILU, p.blockSize,
-                                          blockCrsSize));
+  localPrec_ =
+    Teuchos::rcp(new LocalFILU(skipSortMatrix, this->localRowPtrs_, this->localColInds_, this->localValues_, nRows, p.sptrsv_algo,
+                               p.nFact, p.nTrisol, p.level, p.omega, p.shift, p.guessFlag ? 1 : 0, p.blockSizeILU, p.blockSize,
+                               p.blockCrsSize));
 
   #ifdef HAVE_IFPACK2_METIS
   if (p.use_metis) {

diff --git a/packages/shylu/shylu_node/fastilu/src/shylu_fastilu.hpp b/packages/shylu/shylu_node/fastilu/src/shylu_fastilu.hpp
@@ -78,7 +78,8 @@
 // whether to print timings
 //#define FASTILU_TIMER
 
-// Whether to try to maintain exact behavior with unblocked impl
+// Whether to try to maintain exact behavior with unblocked impl. Comes at
+// a steep performance cost.
 //#define FASTILU_ONE_TO_ONE_UNBLOCKED
 
 template <typename Ordinal>
@@ -1847,7 +1848,7 @@ class FastILUPrec
             }
 
             // sort, if the triangular solve algorithm requires a sorted matrix.
-            bool sortRequired = true; //sptrsv_algo != FastILU::SpTRSV::Fast && sptrsv_algo != FastILU::SpTRSV::StandardHost;
+            bool sortRequired = sptrsv_algo != FastILU::SpTRSV::Fast && sptrsv_algo != FastILU::SpTRSV::StandardHost;
             if (sortRequired) {
               if (blockCrsSize == 1) {
                 KokkosSparse::sort_crs_matrix<ExecSpace, OrdinalArray, OrdinalArray, ScalarArray>