Hypre interface: Clang tidying

Many small things. Replaced some raw pointers with shared_ptr.

Added `modernize-use-ranges` to `.clangd`

Author: bendudson

.clangd

@@ -2,5 +2,6 @@ Diagnostics:
   MissingIncludes: Strict
   ClangTidy:
     FastCheckFilter: None
+    Add: [modernize-use-ranges]
   Includes:
     IgnoreHeader: ["adios2/.*", "bits/.*", "cpptrace/.*", "petsc.*"]

include/bout/hypre_interface.hxx

@@ -15,23 +15,25 @@
 #include "bout/field2d.hxx"
 #include "bout/globalindexer.hxx"
 #include "bout/hyprelib.hxx"
+#include "bout/msg_stack.hxx"
 #include "bout/options.hxx"
 #include "bout/output.hxx"
 #include "bout/paralleltransform.hxx"
 #include "bout/region.hxx"
+#include "bout/unused.hxx"
 #include "bout/utils.hxx"
 
 #include "HYPRE.h"
 #include "HYPRE_IJ_mv.h"
 #include "HYPRE_parcsr_ls.h"
 #include "HYPRE_parcsr_mv.h"
 #include "HYPRE_utilities.h"
-#include "_hypre_utilities.h"
 
 #include <algorithm>
 #include <cmath>
 #include <iterator>
 #include <memory>
+#include <utility>
 #include <vector>
 
 // BOUT_ENUM_CLASS does not work inside namespaces
@@ -48,7 +50,7 @@ namespace bout {
 
 namespace {
 int checkHypreError(int error) {
-  if (error) {
+  if (error != 0) {
     // Aaron Fisher warns that it is possible that the error code is non-zero even
     // though a solve was successful. If this occurs, we might want to make this a
     // warning, or otherwise investigate further.
@@ -217,7 +219,7 @@ class HypreVector {
   HYPRE_Complex* V{nullptr};
   HYPRE_Complex* workV{nullptr};
   bool cache_current{false};
-  HypreLib hyprelib{};
+  HypreLib hyprelib;
 
 public:
   static_assert(bout::utils::is_Field_v<T>, "HypreVector only works with Fields");
@@ -241,7 +243,7 @@ public:
   HypreVector(const HypreVector<T>&) = delete;
   auto operator=(const HypreVector<T>&) = delete;
 
-  HypreVector(HypreVector<T>&& other)
+  HypreVector(HypreVector<T>&& other) noexcept
       : comm(other.comm), jlower(other.jlower), jupper(other.jupper), vsize(other.vsize),
         indexConverter(other.indexConverter), location(other.location),
         initialised(other.initialised), have_indices(other.have_indices), I(other.I),
@@ -312,7 +314,8 @@ public:
   }
 
   /// Construct a vector with given index set, but don't set any values
-  explicit HypreVector(IndexerPtr<T> indConverter) : indexConverter(indConverter) {
+  explicit HypreVector(IndexerPtr<T> indConverter)
+      : indexConverter(indConverter), location(CELL_LOC::centre) {
     Mesh& mesh = *indConverter->getMesh();
     const MPI_Comm comm =
         std::is_same_v<T, FieldPerp> ? mesh.getXZcomm() : BoutComm::get();
@@ -325,7 +328,6 @@ public:
     checkHypreError(HYPRE_IJVectorSetObjectType(hypre_vector, HYPRE_PARCSR));
     checkHypreError(HYPRE_IJVectorInitialize(hypre_vector));
     initialised = true;
-    location = CELL_LOC::centre;
     HypreMalloc(I, vsize * sizeof(HYPRE_BigInt));
     HypreMalloc(V, vsize * sizeof(HYPRE_Complex));
     HypreMalloc(workV, vsize * sizeof(HYPRE_Complex));
@@ -546,13 +548,13 @@ class HypreMatrix {
   ParallelTransform* parallel_transform{nullptr};
   bool assembled{false};
   HYPRE_BigInt num_rows;
-  std::vector<HYPRE_BigInt>* I;
-  std::vector<std::vector<HYPRE_BigInt>>* J;
-  std::vector<std::vector<HYPRE_Complex>>* V;
+  std::shared_ptr<std::vector<HYPRE_BigInt>> I;
+  std::shared_ptr<std::vector<std::vector<HYPRE_BigInt>>> J;
+  std::shared_ptr<std::vector<std::vector<HYPRE_Complex>>> V;
   /// Enable reduction of boundary equations before assembling the Hypre matrix.
   bool use_boundary_elimination{false};
   BoundaryEliminationPtr boundary_elimination;
-  HypreLib hyprelib{};
+  HypreLib hyprelib;
 
   // todo also take care of I,J,V
   struct MatrixDeleter {
@@ -575,8 +577,8 @@ public:
         index_converter(other.index_converter), location(other.location),
         initialised(other.initialised), yoffset(other.yoffset),
         parallel_transform(other.parallel_transform), assembled(other.assembled),
-        num_rows(other.num_rows), I(other.I), J(other.J), V(other.V),
-        use_boundary_elimination(other.use_boundary_elimination),
+        num_rows(other.num_rows), I(std::move(other.I)), J(std::move(other.J)),
+        V(std::move(other.V)), use_boundary_elimination(other.use_boundary_elimination),
         boundary_elimination(other.boundary_elimination) {
     std::swap(hypre_matrix, other.hypre_matrix);
     std::swap(parallel_matrix, other.parallel_matrix);
@@ -595,9 +597,9 @@ public:
     parallel_transform = other.parallel_transform;
     assembled = other.assembled;
     num_rows = other.num_rows;
-    I = other.I;
-    J = other.J;
-    V = other.V;
+    I = std::move(other.I);
+    J = std::move(other.J);
+    V = std::move(other.V);
     use_boundary_elimination = other.use_boundary_elimination;
     boundary_elimination = other.boundary_elimination;
     return *this;
@@ -608,27 +610,21 @@ public:
   ///
   /// note: preallocate not currently used, but here to match PetscMatrix interface
   explicit HypreMatrix(IndexerPtr<T> indConverter, bool UNUSED(preallocate) = true)
-      : hypre_matrix(new HYPRE_IJMatrix, MatrixDeleter{}), index_converter(indConverter) {
+      : ilower(indConverter->getGlobalStart()),
+        iupper(ilower + indConverter->size() - 1), // inclusive end
+        hypre_matrix(new HYPRE_IJMatrix, MatrixDeleter{}), index_converter(indConverter),
+        num_rows(iupper - ilower + 1),
+        I(std::make_shared<std::vector<HYPRE_BigInt>>(num_rows)),
+        J(std::make_shared<std::vector<std::vector<HYPRE_BigInt>>>(num_rows)),
+        V(std::make_shared<std::vector<std::vector<HYPRE_Complex>>>(num_rows)) {
     Mesh* mesh = indConverter->getMesh();
     const MPI_Comm comm =
         std::is_same_v<T, FieldPerp> ? mesh->getXZcomm() : BoutComm::get();
     parallel_transform = &mesh->getCoordinates()->getParallelTransform();
 
-    ilower = indConverter->getGlobalStart();
-    iupper = ilower + indConverter->size() - 1; // inclusive end
-    num_rows = iupper - ilower + 1;
-
-    I = new std::vector<HYPRE_BigInt>;
-    J = new std::vector<std::vector<HYPRE_BigInt>>;
-    V = new std::vector<std::vector<HYPRE_Complex>>;
-    (*I).resize(num_rows);
-    (*J).resize(num_rows);
-    (*V).resize(num_rows);
     for (HYPRE_BigInt i = 0; i < num_rows; ++i) {
       (*I)[i] = ilower + i;
-      (*J)[i].resize(0);
       (*J)[i].reserve(10);
-      (*V)[i].resize(0);
       (*V)[i].reserve(10);
     }
 
@@ -659,8 +655,8 @@ public:
     Element(HypreMatrix<T>& matrix_, HYPRE_BigInt row_, HYPRE_BigInt column_,
             std::vector<HYPRE_BigInt> positions_ = {},
             std::vector<BoutReal> weights_ = {})
-        : hypre_matrix(matrix_.get()), row(row_), column(column_), positions(positions_),
-          weights(weights_) {
+        : hypre_matrix(matrix_.get()), row(row_), column(column_),
+          positions(std::move(positions_)), weights(std::move(weights_)) {
 #if CHECK > 2
       for (const auto val : weights) {
         ASSERT3(std::isfinite(val));
@@ -1032,7 +1028,7 @@ public:
         const HYPRE_Complex operator_value = elimination_state.interior_values->data[i];
         elimination_state.interior_values->data[i] = alpha * operator_value;
         elimination_state.boundary_values->data[i] =
-            alpha * operator_value
+            (alpha * operator_value)
             + (y_boundary_values != nullptr ? beta * y_boundary_values->data[i] : 0.0);
       }
     }
@@ -1073,7 +1069,7 @@ private:
   HYPRE_Solver precon;
   bool solver_setup{false};
   HYPRE_SOLVER_TYPE solver_type = HYPRE_SOLVER_TYPE::gmres;
-  HypreLib hyprelib{};
+  HypreLib hyprelib;
 
   // Basically a manual vtable for the various implementations of the
   // HYPRE solver methods. By storing function pointers and setting
@@ -1092,14 +1088,13 @@ private:
   decltype(HYPRE_ParCSRGMRESSolve)* solverSolve{nullptr};
 
 public:
-  HypreSystem(Mesh& mesh, Options& options) {
+  HypreSystem(Mesh& mesh, Options& options)
+      : comm(std::is_same_v<T, FieldPerp> ? mesh.getXZcomm() : BoutComm::get()) {
     solver_type = options["hypre_solver_type"]
                       .doc("Type of solver to use when solving Hypre system. Possible "
                            "values are: gmres, bicgstab, pcg")
                       .withDefault(HYPRE_SOLVER_TYPE::bicgstab);
 
-    comm = std::is_same_v<T, FieldPerp> ? mesh.getXZcomm() : BoutComm::get();
-
     auto print_level =
         options["hypre_print_level"]
             .doc("Verbosity for Hypre solver. Integer from 0 (silent) to 4 "

src/sys/hypre_interface.cxx

@@ -3,10 +3,17 @@
 
 #if BOUT_HAS_HYPRE
 
+#include <HYPRE_utilities.h>
+
+#include "bout/boutexception.hxx"
 #include "bout/hypre_interface.hxx"
 
+#include <algorithm>
+#include <iterator>
+#include <memory>
 #include <numeric>
 #include <unordered_map>
+#include <vector>
 
 namespace bout {
 
@@ -15,9 +22,8 @@ BoundaryElimination::BoundaryElimination(HYPRE_Int nrows, HYPRE_Int* ncols,
                                          HYPRE_BigInt* cols, HYPRE_Complex* values,
                                          HYPRE_Int nb, HYPRE_Int* bi_array)
     : nb(nb) {
-  HYPRE_Int* row_indexes;
   const auto find_local_row = [nrows, rows](HYPRE_BigInt row) -> HYPRE_Int {
-    auto row_position = std::lower_bound(rows, rows + nrows, row);
+    auto* row_position = std::lower_bound(rows, rows + nrows, row);
     if ((row_position == rows + nrows) || (*row_position != row)) {
       throw BoutException("Could not find local row {} while constructing boundary "
                           "elimination data",
@@ -27,6 +33,7 @@ BoundaryElimination::BoundaryElimination(HYPRE_Int nrows, HYPRE_Int* ncols,
   };
 
   // Create the row_indexes array
+  HYPRE_Int* row_indexes{nullptr};
   HypreMalloc(row_indexes, sizeof(HYPRE_Int) * nrows);
   row_indexes[0] = 0;
   for (HYPRE_Int i = 1; i < nrows; i++) {
@@ -71,7 +78,7 @@ BoundaryElimination::BoundaryElimination(HYPRE_Int nrows, HYPRE_Int* ncols,
     // Get boundary equation information and adjust boundary equations
     HYPRE_Int i = bi_array[bnum];
     const HYPRE_Int binum = find_local_row(i);
-    HYPRE_Int bcoeffnum = row_indexes[binum];
+    const HYPRE_Int bcoeffnum = row_indexes[binum];
     HYPRE_Complex bii{0.0}, bij{0.0};
     HYPRE_BigInt j = -1;
 
@@ -148,11 +155,11 @@ BoundaryElimination::BoundaryElimination(HYPRE_Int nrows, HYPRE_Int* ncols,
 
   na = static_cast<HYPRE_Int>(aknums.size());
   HypreMalloc(aoffset_array, sizeof(HYPRE_Int) * (nb + 1));
-  std::copy(aoffsets.begin(), aoffsets.end(), aoffset_array);
+  std::ranges::copy(aoffsets, aoffset_array);
   HypreMalloc(aknum_array, sizeof(HYPRE_Int) * na);
-  std::copy(aknums.begin(), aknums.end(), aknum_array);
+  std::ranges::copy(aknums, aknum_array);
   HypreMalloc(aki_array, sizeof(HYPRE_Complex) * na);
-  std::copy(akis.begin(), akis.end(), aki_array);
+  std::ranges::copy(akis, aki_array);
 
   std::vector<HYPRE_Int> dependency_depth(nb, -1);
   const auto get_depth = [&dependency_depth, this](const auto& self,
@@ -174,15 +181,14 @@ BoundaryElimination::BoundaryElimination(HYPRE_Int nrows, HYPRE_Int* ncols,
 
   expansion_order.resize(nb);
   std::iota(expansion_order.begin(), expansion_order.end(), 0);
-  std::sort(expansion_order.begin(), expansion_order.end(),
-            [&dependency_depth](HYPRE_Int lhs, HYPRE_Int rhs) {
-              if (dependency_depth[lhs] != dependency_depth[rhs]) {
-                return dependency_depth[lhs] < dependency_depth[rhs];
-              }
-              return lhs < rhs;
-            });
+  std::ranges::sort(expansion_order, [&dependency_depth](HYPRE_Int lhs, HYPRE_Int rhs) {
+    if (dependency_depth[lhs] != dependency_depth[rhs]) {
+      return dependency_depth[lhs] < dependency_depth[rhs];
+    }
+    return lhs < rhs;
+  });
   reduction_order = expansion_order;
-  std::reverse(reduction_order.begin(), reduction_order.end());
+  std::ranges::reverse(reduction_order);
 
   // Set return arguments
   *row_indexes_ptr = row_indexes;
@@ -216,9 +222,9 @@ BCValuesPtr BoundaryElimination::reduceRightHandSideInPlace(HYPRE_Complex* rhs)
   // Allocate array to store boundary row values
   BCValuesPtr brhs = copyBoundaryRowValues(rhs);
 
-  for (HYPRE_Int bnum : reduction_order) {
+  for (const HYPRE_Int bnum : reduction_order) {
     for (HYPRE_Int anum = aoffset_array[bnum]; anum < aoffset_array[bnum + 1]; anum++) {
-      HYPRE_Int aknum = aknum_array[anum];
+      const HYPRE_Int aknum = aknum_array[anum];
       rhs[aknum] -= aki_array[anum] * brhs->data[bnum] / bii_array[bnum];
     }
     if (bdep_array[bnum] >= 0) {
@@ -232,9 +238,9 @@ BCValuesPtr BoundaryElimination::reduceRightHandSideInPlace(HYPRE_Complex* rhs)
 void BoundaryElimination::expandSolutionInPlace(BCValuesPtr brhs,
                                                 HYPRE_Complex* solution) const {
 
-  for (HYPRE_Int bnum : expansion_order) {
-    HYPRE_Int binum = binum_array[bnum];
-    HYPRE_Int bjnum = bjnum_array[bnum];
+  for (const HYPRE_Int bnum : expansion_order) {
+    const HYPRE_Int binum = binum_array[bnum];
+    const HYPRE_Int bjnum = bjnum_array[bnum];
     solution[binum] =
         (brhs->data[bnum] - bij_array[bnum] * solution[bjnum]) / bii_array[bnum];
   }
@@ -245,7 +251,7 @@ void BoundaryElimination::expandMatvecResultInPlace(BCValuesPtr boundary_operato
                                                     HYPRE_Complex* result) const {
   for (HYPRE_Int bnum = 0; bnum < nb; bnum++) {
     for (HYPRE_Int anum = aoffset_array[bnum]; anum < aoffset_array[bnum + 1]; anum++) {
-      HYPRE_Int aknum = aknum_array[anum];
+      const HYPRE_Int aknum = aknum_array[anum];
       result[aknum] +=
           aki_array[anum] * boundary_operator_values->data[bnum] / bii_array[bnum];
     }