Merge pull request #653 from awslabs/phdum/pre_cpp20

Fix small issues in preparation for C++20

Author: phdum-a

.github/workflows/spack.yml

@@ -77,7 +77,19 @@ jobs:
           toolchain: gcc
           variant: "+openmp+int64~shared"
           run-regression-tests: 'false'
- 
+
+  build-x64-gcc-cxx20:
+    needs: filter
+    if: needs.filter.outputs.test == 'true'
+    runs-on: [self-hosted, x64]
+    steps:
+      - uses: actions/checkout@v6
+      - uses: ./.github/actions/palace-ci
+        with:
+          toolchain: gcc
+          variant: " cxxstd=20"
+          run-regression-tests: 'false'
+
   build-test-x64-llvm:
     needs: filter
     if: needs.filter.outputs.test == 'true'

palace/CMakeLists.txt

@@ -15,9 +15,14 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR)
   message(FATAL_ERROR "In-source builds are prohibited")
 endif()
 
-# C++17 required for std::filesystem, among others
+# C++17 required for std::filesystem, among others. Override via -DCMAKE_CXX_STANDARD=<VALUE>
+if(NOT DEFINED CMAKE_CXX_STANDARD)
+  set(CMAKE_CXX_STANDARD 17)
+endif()
+if(CMAKE_CXX_STANDARD LESS 17)
+  message(FATAL_ERROR "Palace requires C++17 or later (got ${CMAKE_CXX_STANDARD})")
+endif()
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
-set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_EXTENSIONS OFF)
 
 # Initialize the project

palace/drivers/electrostaticsolver.cpp

@@ -145,7 +145,6 @@ void ElectrostaticSolver::PostprocessTerminals(
     return;
   }
   using VT = Units::ValueType;
-  using fmt::format;
 
   // Write capacitance matrix data.
   auto PrintMatrix = [&terminal_sources, this](const std::string &file,
@@ -158,11 +157,12 @@ void ElectrostaticSolver::PostprocessTerminals(
     int j = 0;
     for (const auto &[idx2, data2] : terminal_sources)
     {
-      output.table.insert(format("i2{}", idx2), format("{}[i][{}] {}", name, idx2, unit));
+      output.table.insert(fmt::format("i2{}", idx2),
+                          fmt::format("{}[i][{}] {}", name, idx2, unit));
       // Use the fact that iterator over i and j is the same span.
       output.table["i"] << idx2;
 
-      auto &col = output.table[format("i2{}", idx2)];
+      auto &col = output.table[fmt::format("i2{}", idx2)];
       for (std::size_t i = 0; i < terminal_sources.size(); i++)
       {
         col << mat(i, j) * scale;

palace/drivers/magnetostaticsolver.cpp

@@ -151,7 +151,6 @@ void MagnetostaticSolver::PostprocessTerminals(
   {
     return;
   }
-  using fmt::format;
 
   // Write inductance matrix data.
   auto PrintMatrix = [&surf_j_op, this](const std::string &file, const std::string &name,
@@ -163,11 +162,12 @@ void MagnetostaticSolver::PostprocessTerminals(
     int j = 0;
     for (const auto &[idx2, data2] : surf_j_op)
     {
-      output.table.insert(format("i2{}", idx2), format("{}[i][{}] {}", name, idx2, unit));
+      output.table.insert(fmt::format("i2{}", idx2),
+                          fmt::format("{}[i][{}] {}", name, idx2, unit));
       // Use the fact that iterator over i and j is the same span.
       output.table["i"] << idx2;
 
-      auto &col = output.table[format("i2{}", idx2)];
+      auto &col = output.table[fmt::format("i2{}", idx2)];
       for (std::size_t i = 0; i < surf_j_op.Size(); i++)
       {
         col << mat(i, j) * scale;

palace/linalg/rap.cpp

@@ -934,9 +934,9 @@ std::unique_ptr<ComplexParOperator>
 BuildParSumOperator(std::complex<double> (&&coeff_in)[N],
                     const ComplexParOperator *(&&ops_in)[N], bool set_essential)
 {
-  return BuildParSumOperator(to_array<std::complex<double>>(std::move(coeff_in)),
-                             to_array<const ComplexParOperator *>(std::move(ops_in)),
-                             set_essential);
+  return BuildParSumOperator(
+      palace::to_array<std::complex<double>>(std::move(coeff_in)),
+      palace::to_array<const ComplexParOperator *>(std::move(ops_in)), set_essential);
 }
 
 template <std::size_t N, typename ScalarType, typename OperType>
@@ -953,8 +953,8 @@ BuildParSumOperator(ScalarType (&&coeff_in)[N], const OperType *(&&ops_in)[N],
   std::transform(ops_in, ops_in + N, par_ops.begin(),
                  [](const OperType *op) { return dynamic_cast<const ParOperType *>(op); });
 
-  return BuildParSumOperator(to_array<ScalarType>(std::move(coeff_in)), std::move(par_ops),
-                             set_essential);
+  return BuildParSumOperator(palace::to_array<ScalarType>(std::move(coeff_in)),
+                             std::move(par_ops), set_essential);
 }
 
 // Explicit instantiation.

palace/models/lumpedportoperator.cpp

@@ -361,70 +361,75 @@ void LumpedPortOperator::PrintBoundaryInfo(const IoData &iodata, const mfem::Par
   {
     return;
   }
-
-  fmt::memory_buffer buf{};  // Output buffer & buffer append lambda for cleaner code
-  auto to = [](auto &buf, auto fmt, auto &&...args)
-  { fmt::format_to(std::back_inserter(buf), fmt, std::forward<decltype(args)>(args)...); };
+  fmt::memory_buffer buffer{};
+  auto out = fmt::appender{buffer};
   using VT = Units::ValueType;
 
   // Print out BC info for all port attributes, for both active and inactive ports.
-  to(buf, "\nConfiguring Robin impedance BC for lumped ports at attributes:\n");
+  fmt::format_to(out, "\nConfiguring Robin impedance BC for lumped ports at attributes:\n");
   for (const auto &[idx, data] : ports)
   {
     for (const auto &elem : data.elems)
     {
       for (auto attr : elem->GetAttrList())
       {
-        to(buf, " {:d}:", attr);
+        fmt::format_to(out, " {:d}:", attr);
         if (std::abs(data.R) > 0.0)
         {
           double Rs = data.R * data.GetToSquare(*elem);
-          to(buf, " Rs = {:.3e} Ω/sq,", iodata.units.Dimensionalize<VT::IMPEDANCE>(Rs));
+          fmt::format_to(out, " Rs = {:.3e} Ω/sq,",
+                         iodata.units.Dimensionalize<VT::IMPEDANCE>(Rs));
         }
         if (std::abs(data.L) > 0.0)
         {
           double Ls = data.L * data.GetToSquare(*elem);
-          to(buf, " Ls = {:.3e} H/sq,", iodata.units.Dimensionalize<VT::INDUCTANCE>(Ls));
+          fmt::format_to(out, " Ls = {:.3e} H/sq,",
+                         iodata.units.Dimensionalize<VT::INDUCTANCE>(Ls));
         }
         if (std::abs(data.C) > 0.0)
         {
           double Cs = data.C / data.GetToSquare(*elem);
-          to(buf, " Cs = {:.3e} F/sq,", iodata.units.Dimensionalize<VT::CAPACITANCE>(Cs));
+          fmt::format_to(out, " Cs = {:.3e} F/sq,",
+                         iodata.units.Dimensionalize<VT::CAPACITANCE>(Cs));
         }
-        to(buf, " n = ({:+.1f})\n", fmt::join(mesh::GetSurfaceNormal(mesh, attr), ","));
+        fmt::format_to(out, " n = ({:+.1f})\n",
+                       fmt::join(mesh::GetSurfaceNormal(mesh, attr), ","));
       }
     }
   }
 
   // Print out port info for all active ports.
-  fmt::memory_buffer buf_a{};
+  fmt::memory_buffer buffer_active{};
   for (const auto &[idx, data] : ports)
   {
     if (!data.active)
     {
       continue;
     }
-    to(buf_a, " Index = {:d}: ", idx);
+    std::vector<std::string> active_port_str;
     if (std::abs(data.R) > 0.0)
     {
-      to(buf_a, "R = {:.3e} Ω,", iodata.units.Dimensionalize<VT::IMPEDANCE>(data.R));
+      active_port_str.emplace_back(
+          fmt::format("R = {:.3e} Ω", iodata.units.Dimensionalize<VT::IMPEDANCE>(data.R)));
     }
     if (std::abs(data.L) > 0.0)
     {
-      to(buf_a, "L = {:.3e} H,", iodata.units.Dimensionalize<VT::INDUCTANCE>(data.L));
+      active_port_str.emplace_back(
+          fmt::format("L = {:.3e} H", iodata.units.Dimensionalize<VT::INDUCTANCE>(data.L)));
     }
     if (std::abs(data.C) > 0.0)
     {
-      to(buf_a, "C = {:.3e} F,", iodata.units.Dimensionalize<VT::CAPACITANCE>(data.C));
+      active_port_str.emplace_back(fmt::format(
+          "C = {:.3e} F", iodata.units.Dimensionalize<VT::CAPACITANCE>(data.C)));
     }
-    buf_a.resize(buf_a.size() - 1);  // Remove last ","
-    to(buf_a, "\n");
+    fmt::format_to(fmt::appender{buffer_active}, " Index = {:d}: {}\n", idx,
+                   fmt::join(active_port_str, ", "));
   }
-  if (buf_a.size() > 0)
+  if (buffer_active.size() > 0)
   {
-    to(buf, "\nConfiguring lumped port circuit properties:\n");
-    buf.append(buf_a);
-    buf_a.clear();
+    fmt::format_to(out, "\nConfiguring lumped port circuit properties:\n");
+    buffer.append(buffer_active);
+    buffer_active.clear();
   }
 
   // Print some information for excited lumped ports.
@@ -439,17 +444,18 @@ void LumpedPortOperator::PrintBoundaryInfo(const IoData &iodata, const mfem::Par
     {
       for (auto attr : elem->GetAttrList())
       {
-        to(buf_a, " {:d}: Index = {:d}\n", attr, idx);
+        fmt::format_to(fmt::appender{buffer_active}, " {:d}: Index = {:d}\n", attr, idx);
       }
     }
   }
-  if (buf_a.size() > 0)
+  if (buffer_active.size() > 0)
   {
-    to(buf, "\nConfiguring lumped port excitation source term at attributes:\n");
-    buf.append(buf_a);
+    fmt::format_to(out,
+                   "\nConfiguring lumped port excitation source term at attributes:\n");
+    buffer.append(buffer_active);
   }
 
-  Mpi::Print("{}", fmt::to_string(buf));
+  Mpi::Print("{}", fmt::to_string(buffer));
 }
 
 const LumpedPortData &LumpedPortOperator::GetPort(int idx) const

palace/models/portexcitations.cpp

@@ -18,33 +18,34 @@ namespace palace
 [[nodiscard]] std::string PortExcitations::FmtLog() const
 {
   fmt::memory_buffer buf{};
-  auto to = [&buf](auto f, auto &&...a)  // mini-lambda for cleaner code
-  { fmt::format_to(std::back_inserter(buf), f, std::forward<decltype(a)>(a)...); };
+  auto out = fmt::appender{buf};
 
   int i = 1;
   for (const auto &[idx, ex] : excitations)
   {
-    to("Excitation{} with index {:d} has contributions from:\n",
-       (Size() > 1) ? fmt::format(" {:d}/{:d}", i, Size()) : "", idx);
+    fmt::format_to(out, "Excitation{} with index {:d} has contributions from:\n",
+                   (Size() > 1) ? fmt::format(" {:d}/{:d}", i, Size()) : "", idx);
     if (!ex.lumped_port.empty())
     {
-      to(" Lumped port{} {:2d}\n", (ex.lumped_port.size() > 1) ? "s" : "",
-         fmt::join(ex.lumped_port, " "));
+      fmt::format_to(out, " Lumped port{} {:2d}\n", (ex.lumped_port.size() > 1) ? "s" : "",
+                     fmt::join(ex.lumped_port, " "));
     }
     if (!ex.wave_port.empty())
     {
-      to(" Wave port{} {:2d}\n", (ex.wave_port.size() > 1) ? "s" : "",
-         fmt::join(ex.wave_port, " "));
+      fmt::format_to(out, " Wave port{} {:2d}\n", (ex.wave_port.size() > 1) ? "s" : "",
+                     fmt::join(ex.wave_port, " "));
     }
     if (!ex.current_port.empty())
     {
-      to(" Surface current port{} {:2d}\n", (ex.current_port.size() > 1) ? "s" : "",
-         fmt::join(ex.current_port, " "));
+      fmt::format_to(out, " Surface current port{} {:2d}\n",
+                     (ex.current_port.size() > 1) ? "s" : "",
+                     fmt::join(ex.current_port, " "));
     }
     if (!ex.current_dipole.empty())
     {
-      to(" Current dipole{} {:2d}\n", (ex.current_dipole.size() > 1) ? "s" : "",
-         fmt::join(ex.current_dipole, " "));
+      fmt::format_to(out, " Current dipole{} {:2d}\n",
+                     (ex.current_dipole.size() > 1) ? "s" : "",
+                     fmt::join(ex.current_dipole, " "));
     }
     i++;
   }

palace/models/postoperator.cpp

@@ -1000,7 +1000,6 @@ void PostOperator<solver_t>::MeasureSParameter() const
   // Depends on LumpedPorts, WavePorts.
   if constexpr (solver_t == ProblemType::DRIVEN)
   {
-    using fmt::format;
     using std::complex_literals::operator""i;
 
     // Don't measure S-Matrix unless there is only one excitation per port. Also, we current
@@ -1034,7 +1033,7 @@ void PostOperator<solver_t>::MeasureSParameter() const
       }
 
       Mpi::Print(" {0} = {1:+.3e}{2:+.3e}i, |{0}| = {3:+.3e}, arg({0}) = {4:+.3e}\n",
-                 format("S[{}][{}]", idx, drive_port_idx), vi.S.real(), vi.S.imag(),
+                 fmt::format("S[{}][{}]", idx, drive_port_idx), vi.S.real(), vi.S.imag(),
                  Measurement::Magnitude(vi.S), Measurement::Phase(vi.S));
     }
     for (const auto &[idx, data] : fem_op->GetWavePortOp())
@@ -1055,7 +1054,7 @@ void PostOperator<solver_t>::MeasureSParameter() const
       vi.S *= std::exp(1i * data.kn0 * data.d_offset);
 
       Mpi::Print(" {0} = {1:+.3e}{2:+.3e}i, |{0}| = {3:+.3e}, arg({0}) = {4:+.3e}\n",
-                 format("S[{}][{}]", idx, drive_port_idx), vi.S.real(), vi.S.imag(),
+                 fmt::format("S[{}][{}]", idx, drive_port_idx), vi.S.real(), vi.S.imag(),
                  Measurement::Magnitude(vi.S), Measurement::Phase(vi.S));
     }
   }
@@ -1152,8 +1151,6 @@ void PostOperator<solver_t>::MeasureProbes() const
 #endif
 }
 
-using fmt::format;
-
 template <ProblemType solver_t>
 template <ProblemType U>
 auto PostOperator<solver_t>::MeasureAndPrintAll(int ex_idx, int step,