PR feedback for phdum/new_postpro

CHANGELOG.md

@@ -58,7 +58,7 @@ The format of this changelog is based on
   - Fixed bug in Nastran mesh reader where carriage returns (`\r`) in the mesh file could
     cause a failure to read the mesh.
   - Changed post-processing, so that appropriate measurements are always written to disk as CSV
-    files. This is a breaking change, since users are now required to specify valid a output
+    files. This is a breaking change, since users are now required to specify a valid output
     folder in `config["Problem"]["Output"]`. Previously an empty string `""` would suppress file
     printing. ParaView printing is still controlled and suppressed by the `"Save"` or `"SaveStep"`
     options in the `config["Solver"][...]`.

palace/drivers/drivensolver.cpp

@@ -200,7 +200,7 @@ DrivenSolver::SweepUniform(SpaceOperator &space_op,
       floquet_corr->AddMult(E, B, 1.0 / omega);
     }
 
-    auto total_domain_energy = post_op.MeasurePrintAll(step, E, B, omega);
+    auto total_domain_energy = post_op.MeasureAndPrintAll(step, E, B, omega);
 
     // Calculate and record the error indicators.
     Mpi::Print(" Updating solution error estimates\n");
@@ -384,7 +384,7 @@ DrivenSolver::SweepAdaptive(SpaceOperator &space_op,
       // B = -1/(iω) ∇ x E + 1/ω kp x E
       floquet_corr->AddMult(E, B, 1.0 / omega);
     }
-    post_op.MeasurePrintAll(step, E, B, omega);
+    post_op.MeasureAndPrintAll(step, E, B, omega);
   }
   // Final postprocessing & printing
   BlockTimer bt0(Timer::POSTPRO);

palace/drivers/eigensolver.cpp

@@ -329,7 +329,7 @@ EigenSolver::Solve(const std::vector<std::unique_ptr<Mesh>> &mesh) const
     }
 
     auto total_domain_energy =
-        post_op.MeasurePrintAll(i, E, B, omega, error_abs, error_bkwd, num_conv);
+        post_op.MeasureAndPrintAll(i, E, B, omega, error_abs, error_bkwd, num_conv);
 
     // Calculate and record the error indicators.
     if (i < iodata.solver.eigenmode.n)

palace/drivers/electrostaticsolver.cpp

@@ -79,7 +79,7 @@ ElectrostaticSolver::Solve(const std::vector<std::unique_ptr<Mesh>> &mesh) const
     Grad.AddMult(V[step], E, -1.0);
 
     // Measurement and printing.
-    auto total_domain_energy = post_op.MeasurePrintAll(step, V[step], E, idx);
+    auto total_domain_energy = post_op.MeasureAndPrintAll(step, V[step], E, idx);
 
     // Calculate and record the error indicators.
     Mpi::Print(" Updating solution error estimates\n");

palace/drivers/magnetostaticsolver.cpp

@@ -84,7 +84,7 @@ MagnetostaticSolver::Solve(const std::vector<std::unique_ptr<Mesh>> &mesh) const
     I_inc[step] = data.GetExcitationCurrent();
 
     // Measurement and printing.
-    auto total_domain_energy = post_op.MeasurePrintAll(step, A[step], B, idx);
+    auto total_domain_energy = post_op.MeasureAndPrintAll(step, A[step], B, idx);
 
     // Calculate and record the error indicators.
     Mpi::Print(" Updating solution error estimates\n");

palace/drivers/transientsolver.cpp

@@ -109,7 +109,7 @@ TransientSolver::Solve(const std::vector<std::unique_ptr<Mesh>> &mesh) const
                linalg::Norml2(space_op.GetComm(), E),
                linalg::Norml2(space_op.GetComm(), B));
 
-    auto total_domain_energy = post_op.MeasurePrintAll(step, E, B, t, J_coef(t));
+    auto total_domain_energy = post_op.MeasureAndPrintAll(step, E, B, t, J_coef(t));
 
     // Calculate and record the error indicators.
     Mpi::Print(" Updating solution error estimates\n");

palace/models/postoperator.cpp

@@ -501,7 +501,7 @@ void PostOperator<solver_t>::MeasureDomainFieldEnergy() const
   if constexpr (HasEGridFunction<solver_t>())
   {
     // Use V if it has it rather than E.
-    auto field = V ? *V : *E;
+    auto &field = V ? *V : *E;
     auto energy_raw = dom_post_op.GetElectricFieldEnergy(field);
     measurement_cache.domain_E_field_energy_all =
         units.Dimensionalize<Units::ValueType::ENERGY>(energy_raw);
@@ -528,7 +528,7 @@ void PostOperator<solver_t>::MeasureDomainFieldEnergy() const
 
   if (HasBGridFunction<solver_t>())
   {
-    auto field = A ? *A : *B;
+    auto &field = A ? *A : *B;
     auto energy_raw = dom_post_op.GetMagneticFieldEnergy(field);
     measurement_cache.domain_H_field_energy_all =
         units.Dimensionalize<Units::ValueType::ENERGY>(energy_raw);
@@ -562,7 +562,7 @@ void PostOperator<solver_t>::MeasureDomainFieldEnergy() const
   {
     Mpi::Print(" Field energy H = {:.3e} J\n", measurement_cache.domain_H_field_energy_all);
   }
-  else
+  else if constexpr (solver_t != config::ProblemData::Type::EIGENMODE)
   {
     Mpi::Print(" Field energy E ({:.3e} J) + H ({:.3e} J) = {:.3e} J\n",
                measurement_cache.domain_E_field_energy_all,
@@ -655,7 +655,7 @@ void PostOperator<solver_t>::MeasureLumpedPortsEig() const
                                measurement_cache.lumped_port_capacitor_energy;
     for (const auto &[idx, data] : fem_op->GetLumpedPortOp())
     {
-      // Get previously computed data: should never fail as defind by MeasureLumpedPorts.
+      // Get previously computed data: should never fail as defined by MeasureLumpedPorts.
       auto &vi = measurement_cache.lumped_port_vi.at(idx);
 
       // Resistive Lumped Ports:
@@ -669,7 +669,7 @@ void PostOperator<solver_t>::MeasureLumpedPortsEig() const
       {
         std::complex<double> I_mj = vi.I_RLC[0];
         // Power = 1/2 R_j I_mj².
-        // Note convetions: mean(I²) = (I_r² + I_i²) / 2;
+        // Note conventions: mean(I²) = (I_r² + I_i²) / 2;
         auto resistor_power = 0.5 * std::abs(data.R) * std::real(I_mj * std::conj(I_mj));
         vi.mode_port_kappa = units.Dimensionalize<Units::ValueType::FREQUENCY>(
             std::copysign(resistor_power / energy_electric_all, I_mj.real()));
@@ -757,17 +757,14 @@ void PostOperator<solver_t>::MeasureSParameter() const
     // so can treat them separatley.
     for (const auto &[idx, data] : fem_op->GetLumpedPortOp())
     {
-      // Get previously computed data: should never fail as defind by MeasureLumpedPorts.
+      // Get previously computed data: should never fail as defined by MeasureLumpedPorts.
       auto &vi = measurement_cache.lumped_port_vi.at(idx);
 
       const LumpedPortData &src_data =
           fem_op->GetLumpedPortOp().GetPort(driven_source_index);
-      // const auto it = measurement_cache.lumped_port_vi.find(idx);
       MFEM_VERIFY(src_data.excitation, "Lumped port index "
                                            << driven_source_index
                                            << " is not marked for excitation!");
-      // MFEM_VERIFY(it != measurement_cache.lumped_port_vi.end(),
-      //             "Could not find lumped port when calculating port S-parameters!");
       std::complex<double> S_ij = vi.S;
       if (idx == driven_source_index)
       {
@@ -789,7 +786,7 @@ void PostOperator<solver_t>::MeasureSParameter() const
     }
     for (const auto &[idx, data] : fem_op->GetWavePortOp())
     {
-      // Get previously computed data: should never fail as defind by MeasureWavePorts.
+      // Get previously computed data: should never fail as defined by MeasureWavePorts.
       auto &vi = measurement_cache.wave_port_vi.at(idx);
 
       // Wave port modes are not normalized to a characteristic impedance so no generalized
@@ -914,13 +911,12 @@ void PostOperator<solver_t>::MeasureProbes() const
 }
 
 using fmt::format;
-// **********
 
 template <config::ProblemData::Type solver_t>
 template <config::ProblemData::Type U>
-auto PostOperator<solver_t>::MeasurePrintAll(int step, const ComplexVector &e,
-                                             const ComplexVector &b,
-                                             std::complex<double> omega)
+auto PostOperator<solver_t>::MeasureAndPrintAll(int step, const ComplexVector &e,
+                                                const ComplexVector &b,
+                                                std::complex<double> omega)
     -> std::enable_if_t<U == config::ProblemData::Type::DRIVEN, double>
 {
   BlockTimer bt0(Timer::POSTPRO);
@@ -948,10 +944,11 @@ auto PostOperator<solver_t>::MeasurePrintAll(int step, const ComplexVector &e,
 
 template <config::ProblemData::Type solver_t>
 template <config::ProblemData::Type U>
-auto PostOperator<solver_t>::MeasurePrintAll(int step, const ComplexVector &e,
-                                             const ComplexVector &b,
-                                             std::complex<double> omega, double error_abs,
-                                             double error_bkwd, int num_conv)
+auto PostOperator<solver_t>::MeasureAndPrintAll(int step, const ComplexVector &e,
+                                                const ComplexVector &b,
+                                                std::complex<double> omega,
+                                                double error_abs, double error_bkwd,
+                                                int num_conv)
     -> std::enable_if_t<U == config::ProblemData::Type::EIGENMODE, double>
 {
   BlockTimer bt0(Timer::POSTPRO);
@@ -981,7 +978,7 @@ auto PostOperator<solver_t>::MeasurePrintAll(int step, const ComplexVector &e,
     table.insert(Column("err_back", "Error (Bkwd.)") << error_abs);
     table.insert(Column("err_abs", "Error (Abs.)") << error_bkwd);
     table[0].print_as_int = true;
-    Mpi::Print("{}", table.format_table());
+    Mpi::Print("{}", (step == 0) ? table.format_table() : table.format_row(0));
   }
   MeasureAllImpl();
 
@@ -1000,8 +997,8 @@ auto PostOperator<solver_t>::MeasurePrintAll(int step, const ComplexVector &e,
 
 template <config::ProblemData::Type solver_t>
 template <config::ProblemData::Type U>
-auto PostOperator<solver_t>::MeasurePrintAll(int step, const Vector &v, const Vector &e,
-                                             int idx)
+auto PostOperator<solver_t>::MeasureAndPrintAll(int step, const Vector &v, const Vector &e,
+                                                int idx)
     -> std::enable_if_t<U == config::ProblemData::Type::ELECTROSTATIC, double>
 {
   BlockTimer bt0(Timer::POSTPRO);
@@ -1026,8 +1023,8 @@ auto PostOperator<solver_t>::MeasurePrintAll(int step, const Vector &v, const Ve
 }
 template <config::ProblemData::Type solver_t>
 template <config::ProblemData::Type U>
-auto PostOperator<solver_t>::MeasurePrintAll(int step, const Vector &a, const Vector &b,
-                                             int idx)
+auto PostOperator<solver_t>::MeasureAndPrintAll(int step, const Vector &a, const Vector &b,
+                                                int idx)
     -> std::enable_if_t<U == config::ProblemData::Type::MAGNETOSTATIC, double>
 {
   BlockTimer bt0(Timer::POSTPRO);
@@ -1053,8 +1050,8 @@ auto PostOperator<solver_t>::MeasurePrintAll(int step, const Vector &a, const Ve
 
 template <config::ProblemData::Type solver_t>
 template <config::ProblemData::Type U>
-auto PostOperator<solver_t>::MeasurePrintAll(int step, const Vector &e, const Vector &b,
-                                             double t, double J_coef)
+auto PostOperator<solver_t>::MeasureAndPrintAll(int step, const Vector &e, const Vector &b,
+                                                double t, double J_coef)
     -> std::enable_if_t<U == config::ProblemData::Type::TRANSIENT, double>
 {
   BlockTimer bt0(Timer::POSTPRO);
@@ -1119,26 +1116,26 @@ template class PostOperator<config::ProblemData::Type::TRANSIENT>;
 // Function explict instantiation.
 // TODO(C++20): with requires, we won't need a second template
 
-template auto PostOperator<config::ProblemData::Type::DRIVEN>::MeasurePrintAll<
+template auto PostOperator<config::ProblemData::Type::DRIVEN>::MeasureAndPrintAll<
     config::ProblemData::Type::DRIVEN>(int step, const ComplexVector &e,
                                        const ComplexVector &b, std::complex<double> omega)
     -> double;
 
-template auto PostOperator<config::ProblemData::Type::EIGENMODE>::MeasurePrintAll<
+template auto PostOperator<config::ProblemData::Type::EIGENMODE>::MeasureAndPrintAll<
     config::ProblemData::Type::EIGENMODE>(int step, const ComplexVector &e,
                                           const ComplexVector &b,
                                           std::complex<double> omega, double error_abs,
                                           double error_bkwd, int num_conv) -> double;
 
-template auto PostOperator<config::ProblemData::Type::ELECTROSTATIC>::MeasurePrintAll<
+template auto PostOperator<config::ProblemData::Type::ELECTROSTATIC>::MeasureAndPrintAll<
     config::ProblemData::Type::ELECTROSTATIC>(int step, const Vector &v, const Vector &e,
                                               int idx) -> double;
 
-template auto PostOperator<config::ProblemData::Type::MAGNETOSTATIC>::MeasurePrintAll<
+template auto PostOperator<config::ProblemData::Type::MAGNETOSTATIC>::MeasureAndPrintAll<
     config::ProblemData::Type::MAGNETOSTATIC>(int step, const Vector &a, const Vector &b,
                                               int idx) -> double;
 
-template auto PostOperator<config::ProblemData::Type::TRANSIENT>::MeasurePrintAll<
+template auto PostOperator<config::ProblemData::Type::TRANSIENT>::MeasureAndPrintAll<
     config::ProblemData::Type::TRANSIENT>(int step, const Vector &e, const Vector &b,
                                           double t, double J_coef) -> double;