Merge Pull Request trilinos#4949 from trilinos/Trilinos/Tpetra-hide-d…

…eprecated-triangular-methods

Automatically Merged using Trilinos Pull Request AutoTester
PR Title: Tpetra: Hide deprecated triangular methods
PR Author: mhoemmen

packages/ifpack2/src/Ifpack2_Details_RowMatrix.hpp

@@ -71,10 +71,10 @@ class RowMatrix :
 public:
   //! \name Typedefs
   //@{
-  typedef typename MatrixType::scalar_type scalar_type;
-  typedef typename MatrixType::local_ordinal_type local_ordinal_type;
-  typedef typename MatrixType::global_ordinal_type global_ordinal_type;
-  typedef typename MatrixType::node_type node_type;
+  using scalar_type = typename MatrixType::scalar_type;
+  using local_ordinal_type = typename MatrixType::local_ordinal_type;
+  using global_ordinal_type = typename MatrixType::global_ordinal_type;
+  using node_type = typename MatrixType::node_type;
 
   //@}
   //! \name Destructor
@@ -84,13 +84,14 @@ class RowMatrix :
   virtual ~RowMatrix () = default;
 
   //@}
+
+#ifdef TPETRA_ENABLE_DEPRECATED_CODE
   /// \name Work-around implementations of deprecated virtual methods
   ///
   /// These methods exist to smooth the path for fixing GitHub Issue
   /// #2630.  This is why their existence depends on a Tpetra macro.
   //@{
 
-#ifdef TPETRA_ENABLE_DEPRECATED_CODE
   /// \brief The global number of diagonal entries.
   ///
   /// \warning This method is DEPRECATED and will be removed soon!
@@ -129,6 +130,8 @@ class RowMatrix :
     throwBecauseDeprecated ("isUpperTriangular");
     return false;
   }
+
+  //@}
 #endif // TPETRA_ENABLE_DEPRECATED_CODE
 };
 

packages/ifpack2/src/Ifpack2_LocalSparseTriangularSolver_def.hpp

@@ -363,9 +363,11 @@ void
 LocalSparseTriangularSolver<MatrixType>::
 initialize ()
 {
+  using Tpetra::Details::determineLocalTriangularStructure;
   using crs_matrix_type = Tpetra::CrsMatrix<scalar_type, local_ordinal_type,
     global_ordinal_type, node_type>;
   using local_matrix_type = typename crs_matrix_type::local_matrix_type;
+  using LO = local_ordinal_type;
 
   const char prefix[] = "Ifpack2::LocalSparseTriangularSolver::initialize: ";
   if (! out_.is_null ()) {
@@ -395,14 +397,26 @@ initialize ()
     (! G->isFillComplete (), std::runtime_error, "If you call this method, "
      "the matrix's graph must be fill complete.  It is not.");
 
-  // FIXME (mfh 01,02 Jun 2018) isUpperTriangular has been DEPRECATED.
-  // See GitHub Issue #2630.  I'm using isUpperTriangularImpl ONLY to
-  // avoid deprecated warnings.  Users may NOT call this method.
-  //
-  // FIXME (mfh 02 Jun 2018) Move the
-  // determineLocalTriangularStructure call above this test, so we can
-  // use that result, rather than the deprecated method.
-  if (reverseStorage_ && A_crs_->isUpperTriangularImpl() && htsImpl_.is_null()) {
+  // mfh 30 Apr 2018: See GitHub Issue #2658.
+  constexpr bool ignoreMapsForTriStructure = true;
+  auto lclTriStructure = [&] {
+    auto lclMatrix = A_crs_->getLocalMatrix ();
+    auto lclRowMap = A_crs_->getRowMap ()->getLocalMap ();
+    auto lclColMap = A_crs_->getColMap ()->getLocalMap ();
+    auto lclTriStruct =
+      determineLocalTriangularStructure (lclMatrix.graph,
+                                         lclRowMap,
+                                         lclColMap,
+                                         ignoreMapsForTriStructure);
+    const LO lclNumRows = lclRowMap.getNodeNumElements ();
+    this->diag_ = (lclTriStruct.diagCount < lclNumRows) ? "U" : "N";
+    this->uplo_ = lclTriStruct.couldBeLowerTriangular ? "L" :
+      (lclTriStruct.couldBeUpperTriangular ? "U" : "N");
+    return lclTriStruct;
+  } ();
+
+  if (reverseStorage_ && lclTriStructure.couldBeUpperTriangular &&
+      htsImpl_.is_null ()) {
     // Reverse the storage for an upper triangular matrix
     auto Alocal = A_crs_->getLocalMatrix();
     auto ptr    = Alocal.graph.row_map;
@@ -467,6 +481,19 @@ initialize ()
     A_crs_ = Teuchos::rcp(new crs_matrix_type(newLocalMatrix, newRowMap, newColMap, A_crs_->getDomainMap(), A_crs_->getRangeMap()));
 
     isInternallyChanged_ = true;
+
+    // FIXME (mfh 18 Apr 2019) Recomputing this is unnecessary, but I
+    // didn't want to break any invariants, especially considering
+    // that this branch is likely poorly tested.
+    auto newLclTriStructure =
+      determineLocalTriangularStructure (newLocalMatrix.graph,
+                                         newRowMap->getLocalMap (),
+                                         newColMap->getLocalMap (),
+                                         ignoreMapsForTriStructure);
+    const LO newLclNumRows = newRowMap->getNodeNumElements ();
+    this->diag_ = (newLclTriStructure.diagCount < newLclNumRows) ? "U" : "N";
+    this->uplo_ = newLclTriStructure.couldBeLowerTriangular ? "L" :
+      (newLclTriStructure.couldBeUpperTriangular ? "U" : "N");
   }
 
   if (Teuchos::nonnull (htsImpl_))
@@ -475,21 +502,6 @@ initialize ()
     isInternallyChanged_ = true;
   }
 
-  auto lclMatrix = A_crs_->getLocalMatrix ();
-  auto lclRowMap = A_crs_->getRowMap ()->getLocalMap ();
-  auto lclColMap = A_crs_->getColMap ()->getLocalMap ();
-  using Tpetra::Details::determineLocalTriangularStructure;
-  // mfh 30 Apr 2018: See GitHub Issue #2658 for why this is false.
-  constexpr bool ignoreMapsForTriangularStructure = true;
-  auto result =
-    determineLocalTriangularStructure (lclMatrix.graph, lclRowMap, lclColMap,
-                                       ignoreMapsForTriangularStructure);
-  using LO = local_ordinal_type;
-  const LO lclNumRows = lclRowMap.getNodeNumElements ();
-  this->diag_ = (result.diagCount < lclNumRows) ? "U" : "N";
-  this->uplo_ = result.couldBeLowerTriangular ? "L" :
-    (result.couldBeUpperTriangular ? "U" : "N");
-
   isInitialized_ = true;
   ++numInitialize_;
 }

packages/ifpack2/test/unit_tests/Ifpack2_UnitTestRelaxation.cpp

@@ -305,14 +305,6 @@ TEUCHOS_UNIT_TEST_TEMPLATE_3_DECL(Ifpack2Relaxation, SymGaussSeidelZeroRows, Sca
     return;
   }
 
-  // Create the Node instance.  This ensures that the test will work
-  // even if Node is not the default Node type.
-  RCP<Node> node;
-  { // All Node constructors demand a ParameterList input.
-    ParameterList junk;
-    node = rcp (new Node (junk));
-  }
-
   // The number of rows of the matrix and vectors owned by the calling
   // process.  One process (Proc 0) owns zero rows, and the other
   // process(es) own a nonzero amount of rows.  This is the point of

packages/tpetra/core/src/Tpetra_CrsGraph_decl.hpp

@@ -75,9 +75,9 @@ namespace Tpetra {
   // Forward declaration for CrsGraph::swap() test
   template<class LocalOrdinal, class GlobalOrdinal, class Node> class crsGraph_Swap_Tester;
 
-
 #ifndef DOXYGEN_SHOULD_SKIP_THIS
   namespace Details {
+#ifdef TPETRA_ENABLE_DEPRECATED_CODE
     // Forward declaration of an implementation detail of CrsGraph::clone.
     template<class OutputCrsGraphType, class InputCrsGraphType>
     class CrsGraphCopier {
@@ -87,6 +87,7 @@ namespace Tpetra {
              const Teuchos::RCP<typename OutputCrsGraphType::node_type> nodeOut,
              const Teuchos::RCP<Teuchos::ParameterList>& params = Teuchos::null);
     };  // class CrsGraphCopier
+#endif // TPETRA_ENABLE_DEPRECATED_CODE
 
     template<class LO, class GO, class NT>
     void
@@ -185,7 +186,7 @@ namespace Tpetra {
       STORAGE_UB //<! Invalid value; upper bound on enum values
     };
 
-
+#ifdef TPETRA_ENABLE_DEPRECATED_CODE
     /// \brief Mix-in to avoid spurious deprecation warnings due to #2630.
     ///
     /// CrsMatrix has methods deprecated by #2630, that need to call
@@ -196,12 +197,13 @@ namespace Tpetra {
     /// deprecated methods without emitting spurious warnings.
     class HasDeprecatedMethods2630_WarningThisClassIsNotForUsers {
     public:
-      virtual ~HasDeprecatedMethods2630_WarningThisClassIsNotForUsers () {}
+      virtual ~HasDeprecatedMethods2630_WarningThisClassIsNotForUsers () = default;
       virtual bool isLowerTriangularImpl () const = 0;
       virtual bool isUpperTriangularImpl () const = 0;
       virtual size_t getGlobalNumDiagsImpl () const = 0;
       virtual global_size_t getNodeNumDiagsImpl () const = 0;
     };  // class HasDeprecatedMethods2630_WarningThisClassIsNotForUsers
+#endif // TPETRA_ENABLE_DEPRECATED_CODE
   } // namespace Details
 
   /// \class CrsGraph
@@ -271,8 +273,10 @@ namespace Tpetra {
                       LocalOrdinal,
                       GlobalOrdinal,
                       Node>,
-    public Teuchos::ParameterListAcceptorDefaultBase,
-    public ::Tpetra::Details::HasDeprecatedMethods2630_WarningThisClassIsNotForUsers
+    public Teuchos::ParameterListAcceptorDefaultBase
+#ifdef TPETRA_ENABLE_DEPRECATED_CODE
+    , public ::Tpetra::Details::HasDeprecatedMethods2630_WarningThisClassIsNotForUsers
+#endif // TPETRA_ENABLE_DEPRECATED_CODE
   {
     template <class S, class LO, class GO, class N>
     friend class CrsMatrix;
@@ -1078,16 +1082,15 @@ namespace Tpetra {
     ///
     /// \warning This method is DEPRECATED.  DO NOT CALL IT.  It may
     ///   go away at any time.
-    global_size_t TPETRA_DEPRECATED getGlobalNumDiags() const override;
+    global_size_t TPETRA_DEPRECATED getGlobalNumDiags () const override;
 
     /// \brief Number of diagonal entries on the calling process.
     ///
     /// \pre <tt>! this->isFillActive()</tt>
     ///
     /// \warning This method is DEPRECATED.  DO NOT CALL IT.  It may
     ///   go away at any time.
-    size_t TPETRA_DEPRECATED getNodeNumDiags() const override;
-#endif // TPETRA_ENABLE_DEPRECATED_CODE
+    size_t TPETRA_DEPRECATED getNodeNumDiags () const override;
 
     /// \brief DO NOT CALL THIS METHOD; THIS IS NOT FOR USERS.
     ///
@@ -1115,7 +1118,6 @@ namespace Tpetra {
     /// methods, not if <i>we</i> call them.
     size_t getNodeNumDiagsImpl () const override;
 
-#ifdef TPETRA_ENABLE_DEPRECATED_CODE
     /// \brief Whether the graph is locally lower triangular.
     ///
     /// \warning DO NOT CALL THIS METHOD!  This method is DEPRECATED
@@ -1139,7 +1141,6 @@ namespace Tpetra {
     /// \note This is entirely a local property.  That means this
     ///   method may return different results on different processes.
     bool TPETRA_DEPRECATED isUpperTriangular () const override;
-#endif // TPETRA_ENABLE_DEPRECATED_CODE
 
     /// \brief DO NOT CALL THIS METHOD; THIS IS NOT FOR USERS.
     ///
@@ -1166,17 +1167,30 @@ namespace Tpetra {
     /// to see deprecated warnings if <i>they</i> call deprecated
     /// methods, not if <i>we</i> call them.
     bool isUpperTriangularImpl () const override;
+#endif // TPETRA_ENABLE_DEPRECATED_CODE
 
-    //! \brief If graph indices are in the local range, this function returns true. Otherwise, this function returns false. */
+    /// \brief Whether the graph's column indices are stored as local indices.
+    ///
+    /// Weird quirk inherited from Epetra:
+    /// <tt>! isLocallyIndexed() && ! isGloballyIndexed()</tt>
+    /// means that there are no graph entries on the calling process.
+    /// Please don't rely on this behavior, but note that it's
+    /// possible.
     bool isLocallyIndexed () const override;
 
-    //! \brief If graph indices are in the global range, this function returns true. Otherwise, this function returns false. */
+    /// \brief Whether the graph's column indices are stored as global indices.
+    ///
+    /// Weird quirk inherited from Epetra:
+    /// <tt>! isLocallyIndexed() && ! isGloballyIndexed()</tt>
+    /// means that there are no graph entries on the calling process.
+    /// Please don't rely on this behavior, but note that it's
+    /// possible.
     bool isGloballyIndexed () const override;
 
-    //! Returns \c true if fillComplete() has been called and the graph is in compute mode.
+    //! Whether fillComplete() has been called and the graph is in compute mode.
     bool isFillComplete () const override;
 
-    //! Returns \c true if resumeFill() has been called and the graph is in edit mode.
+    //! Whether resumeFill() has been called and the graph is in edit mode.
     bool isFillActive () const;
 
     /// \brief Whether graph indices in all rows are known to be sorted.
@@ -1202,25 +1216,25 @@ namespace Tpetra {
 
     /// \brief Get a copy of the given row, using global indices.
     ///
-    /// \param GlobalRow [in] Global index of the row.
-    /// \param Indices [out] On output: Global column indices.
-    /// \param NumIndices [out] Number of indices returned.
+    /// \param gblRow [in] Global index of the row.
+    /// \param gblColInds [out] On output: Global column indices.
+    /// \param numColInds [out] Number of indices returned.
     void
-    getGlobalRowCopy (global_ordinal_type GlobalRow,
-                      const Teuchos::ArrayView<global_ordinal_type>& Indices,
-                      size_t& NumIndices) const override;
+    getGlobalRowCopy (global_ordinal_type gblRow,
+                      const Teuchos::ArrayView<global_ordinal_type>& gblColInds,
+                      size_t& numColInds) const override;
 
     /// \brief Get a copy of the given row, using local indices.
     ///
-    /// \param LocalRow [in] Local index of the row.
-    /// \param Indices [out] On output: Local column indices.
-    /// \param NumIndices [out] Number of indices returned.
+    /// \param lclRow [in] Local index of the row.
+    /// \param lclColInds [out] On output: Local column indices.
+    /// \param numColInds [out] Number of indices returned.
     ///
     /// \pre <tt>hasColMap()</tt>
     void
-    getLocalRowCopy (local_ordinal_type LocalRow,
-                     const Teuchos::ArrayView<local_ordinal_type>& indices,
-                     size_t& NumIndices) const override;
+    getLocalRowCopy (local_ordinal_type lclRow,
+                     const Teuchos::ArrayView<local_ordinal_type>& lclColInds,
+                     size_t& numColInds) const override;
 
     /// \brief Get a const, non-persisting view of the given global
     ///   row's global column indices, as a Teuchos::ArrayView.
@@ -2812,8 +2826,8 @@ namespace Tpetra {
     return destGraph;
   }
 
+#ifdef TPETRA_ENABLE_DEPRECATED_CODE
   namespace Details {
-
     template<class LocalOrdinal,
              class GlobalOrdinal,
              class OutputNodeType,
@@ -3212,8 +3226,9 @@ namespace Tpetra {
         return clonedGraph;
       }
     };  // class CrsGraphCopier
-
   } // namespace Details
+#endif // TPETRA_ENABLE_DEPRECATED_CODE
+
 } // namespace Tpetra
 
 #endif // TPETRA_CRSGRAPH_DECL_HPP

packages/tpetra/core/src/Tpetra_CrsGraph_def.hpp

@@ -1000,7 +1000,6 @@ namespace Tpetra {
   {
     return this->getNodeNumDiagsImpl ();
   }
-#endif // TPETRA_ENABLE_DEPRECATED_CODE
 
   template <class LocalOrdinal, class GlobalOrdinal, class Node>
   global_size_t
@@ -1023,6 +1022,7 @@ namespace Tpetra {
   {
     return nodeNumDiags_;
   }
+#endif // TPETRA_ENABLE_DEPRECATED_CODE
 
   template <class LocalOrdinal, class GlobalOrdinal, class Node>
   Teuchos::RCP<Node>
@@ -1240,7 +1240,6 @@ namespace Tpetra {
   {
     return this->isUpperTriangularImpl ();
   }
-#endif // TPETRA_ENABLE_DEPRECATED_CODE
 
   template <class LocalOrdinal, class GlobalOrdinal, class Node>
   bool
@@ -1257,6 +1256,7 @@ namespace Tpetra {
   {
     return this->upperTriangular_;
   }
+#endif // TPETRA_ENABLE_DEPRECATED_CODE
 
   template <class LocalOrdinal, class GlobalOrdinal, class Node>
   bool