From 83b6cda2da56b2879cd3eed65ce41593e8d24783 Mon Sep 17 00:00:00 2001 From: Remi Lehe Date: Wed, 2 Sep 2026 06:48:32 -0700 Subject: [PATCH 1/9] Remove the unused vector argument from Preconditioner::Update None of the preconditioners re-linearize about the vector passed to `Update()`, so the argument only suggests a dependence that does not exist. Of the three implementations, `CurlCurlMLMGPC` ignored it outright, while `JacobiPC` and `MatrixPC` used it purely for layout and index metadata, never for the field values: - `JacobiPC` read `boxArray()`/`DistributionMap()` to size the scratch MultiFabs it allocates lazily. These are now cached in `Define()`, which already receives the same vector. The allocation stays lazy: its guard is on the mass-matrix coefficients being set, not on the vector. - `MatrixPC::Assemble()` read `getDOFsObject()`. The solver vector class owns a single DOF object that lives until AMReX finalization, so this is now cached in `Define()` as a non-owning pointer as well. The state a preconditioner actually depends on reaches it through the `Ops` object and through `CurTime()`/`CurTimeStep()`, so dropping the argument makes the real data flow explicit. Should a preconditioner ever need the current iterate, it can be added back at that point. `LinearFunction::updatePreCondMat()` exists only to forward to `Update()`, so it loses its argument too, along with its call sites in `NewtonSolver`, `WarpX_PETSc` and `DarwinLinearFieldOperator`. Also drop a stale `ignore_unused(a_U)` in `MatrixPC::Define()`, whose `// a_U is not needed` comment contradicted the three uses of `a_U` immediately below it. Co-Authored-By: Claude Opus 5 --- .../DarwinLinearFieldOperator.H | 5 +-- Source/NonlinearSolvers/CurlCurlMLMGPC.H | 7 +--- Source/NonlinearSolvers/JacobiPC.H | 36 ++++++++++++----- Source/NonlinearSolvers/JacobianFunctionMF.H | 4 +- Source/NonlinearSolvers/LinearFunction.H | 2 +- Source/NonlinearSolvers/MatrixPC.H | 40 +++++++++++++------ Source/NonlinearSolvers/NewtonSolver.H | 2 +- Source/NonlinearSolvers/Preconditioner.H | 8 +++- Source/NonlinearSolvers/WarpX_PETSc.cpp | 2 +- 9 files changed, 67 insertions(+), 39 deletions(-) diff --git a/Source/FieldSolver/ImplicitSolvers/DarwinLinearFieldOperator.H b/Source/FieldSolver/ImplicitSolvers/DarwinLinearFieldOperator.H index 3976e0cebb1..b2bc3a1a748 100644 --- a/Source/FieldSolver/ImplicitSolvers/DarwinLinearFieldOperator.H +++ b/Source/FieldSolver/ImplicitSolvers/DarwinLinearFieldOperator.H @@ -69,10 +69,7 @@ public: } inline - void updatePreCondMat ( const WarpXSolverVec& a_X ) override - { - amrex::ignore_unused(a_X); - } + void updatePreCondMat () override { } inline void getPCMatrix ( amrex::Gpu::DeviceVector& a_ridx_g, diff --git a/Source/NonlinearSolvers/CurlCurlMLMGPC.H b/Source/NonlinearSolvers/CurlCurlMLMGPC.H index 514d97a1b7a..8ba6262426d 100644 --- a/Source/NonlinearSolvers/CurlCurlMLMGPC.H +++ b/Source/NonlinearSolvers/CurlCurlMLMGPC.H @@ -81,7 +81,7 @@ class CurlCurlMLMGPC : public Preconditioner /** * \brief Update the preconditioner */ - void Update (const T& a_U) override; + void Update () override; /** * \brief Apply (solve) the preconditioner given a RHS @@ -247,7 +247,7 @@ void CurlCurlMLMGPC::Define ( const T& a_U, } template -void CurlCurlMLMGPC::Update (const T& a_U) +void CurlCurlMLMGPC::Update () { BL_PROFILE("CurlCurlMLMGPC::Update()"); using namespace amrex; @@ -256,9 +256,6 @@ void CurlCurlMLMGPC::Update (const T& a_U) IsDefined(), "CurlCurlMLMGPC::Update() called on undefined object" ); - // a_U is not needed for a linear operator - amrex::ignore_unused(a_U); - // set the alpha coefficient for the curl-curl op const RT thetaDt = m_ops->GetThetaForPC()*this->m_dt; if (thetaDt==0.) { diff --git a/Source/NonlinearSolvers/JacobiPC.H b/Source/NonlinearSolvers/JacobiPC.H index c669c295dcf..2ccfae3bddb 100644 --- a/Source/NonlinearSolvers/JacobiPC.H +++ b/Source/NonlinearSolvers/JacobiPC.H @@ -72,7 +72,7 @@ class JacobiPC : public Preconditioner void Define (const T&, Ops*) override; - void Update (const T& a_U) override; + void Update () override; /** * \brief Solve (I + M) x = b via damped Jacobi iteration @@ -110,6 +110,13 @@ class JacobiPC : public Preconditioner int m_num_amr_levels = 0; + /** + * \brief Grid layout of the solver vector, cached in Define() so that + * Update() can allocate its scratch without being handed a vector. + */ + amrex::Vector> m_grids; + amrex::Vector> m_dmap; + const amrex::Vector>* m_bcoefs = nullptr; bool m_has_offdiag = false; @@ -269,13 +276,26 @@ void JacobiPC::Define ( const T& a_U, m_num_amr_levels = m_ops->numAMRLevels(); m_bcoefs = m_ops->GetMassMatricesCoeff(); + // Cache the grid layout of the solver vector. The scratch MultiFabs are + // allocated lazily in Update(), once the mass-matrix coefficients are + // known, but their layout is already fixed here. + const auto& u_mfarrvec = a_U.getArrayVec(); + m_grids.resize(m_num_amr_levels); + m_dmap.resize(m_num_amr_levels); + for (int n = 0; n < m_num_amr_levels; n++) { + for (int dim = 0; dim < 3; dim++) { + m_grids[n][dim] = u_mfarrvec[n][dim]->boxArray(); + m_dmap[n][dim] = u_mfarrvec[n][dim]->DistributionMap(); + } + } + readParameters(); m_is_defined = true; } template -void JacobiPC::Update (const T& a_U) +void JacobiPC::Update () { BL_PROFILE("JacobiPC::Update()"); using namespace amrex; @@ -285,7 +305,6 @@ void JacobiPC::Update (const T& a_U) "JacobiPC::Update() called on undefined object" ); if (m_bcoefs != nullptr && !m_work_defined) { - auto& u_mfarrvec = a_U.getArrayVec(); m_work.resize(m_num_amr_levels); m_x_ghost.resize(m_num_amr_levels); @@ -307,15 +326,10 @@ void JacobiPC::Update (const T& a_U) const amrex::IntVect nghost(m_stencil_width); for (int n = 0; n < m_num_amr_levels; n++) { for (int dim = 0; dim < 3; dim++) { - m_work[n][dim].define( - u_mfarrvec[n][dim]->boxArray(), - u_mfarrvec[n][dim]->DistributionMap(), - 1, 0); + m_work[n][dim].define(m_grids[n][dim], m_dmap[n][dim], 1, 0); if (m_has_offdiag) { - m_x_ghost[n][dim].define( - u_mfarrvec[n][dim]->boxArray(), - u_mfarrvec[n][dim]->DistributionMap(), - 1, nghost); + m_x_ghost[n][dim].define(m_grids[n][dim], m_dmap[n][dim], + 1, nghost); } } } diff --git a/Source/NonlinearSolvers/JacobianFunctionMF.H b/Source/NonlinearSolvers/JacobianFunctionMF.H index af9d3b2e999..5a34257ea27 100644 --- a/Source/NonlinearSolvers/JacobianFunctionMF.H +++ b/Source/NonlinearSolvers/JacobianFunctionMF.H @@ -52,9 +52,9 @@ class JacobianFunctionMF : public LinearFunction } inline - void updatePreCondMat ( const T& a_X ) override + void updatePreCondMat () override { - if (m_usePreCond) { m_preCond->Update(a_X); } + if (m_usePreCond) { m_preCond->Update(); } } inline diff --git a/Source/NonlinearSolvers/LinearFunction.H b/Source/NonlinearSolvers/LinearFunction.H index 41cd71ab308..eaaf44b9d6f 100644 --- a/Source/NonlinearSolvers/LinearFunction.H +++ b/Source/NonlinearSolvers/LinearFunction.H @@ -58,7 +58,7 @@ class LinearFunction virtual void precond ( T& a_U, const T& a_X ) = 0; //! update preconditioner - virtual void updatePreCondMat ( const T& a_X ) = 0; + virtual void updatePreCondMat () = 0; //! get sparse matrix representation of preconditioner virtual void getPCMatrix( amrex::Gpu::DeviceVector&, diff --git a/Source/NonlinearSolvers/MatrixPC.H b/Source/NonlinearSolvers/MatrixPC.H index ad9d431d442..a73897bbf01 100644 --- a/Source/NonlinearSolvers/MatrixPC.H +++ b/Source/NonlinearSolvers/MatrixPC.H @@ -21,6 +21,8 @@ #include #include +#include + namespace MatrixPCUtils { AMREX_GPU_HOST_DEVICE AMREX_FORCE_INLINE @@ -104,7 +106,7 @@ class MatrixPC : public Preconditioner /** * \brief Update the preconditioner */ - void Update (const T& a_U) override; + void Update () override; /** * \brief Assemble the matrix @@ -114,7 +116,7 @@ class MatrixPC : public Preconditioner * non-zero elements (return value is difference in current number * of nonzero elements and the desired number). */ - int Assemble (const T& a_U); + int Assemble (); /** * \brief Apply (solve) the preconditioner given a RHS @@ -185,6 +187,15 @@ class MatrixPC : public Preconditioner int m_ndofs_l = 0; int m_ndofs_g = 0; + + /** + * \brief The DOF numbering object of the solver vector, cached in + * Define(). The solver vector class owns a single instance of it that + * lives until AMReX finalization, so a non-owning pointer is safe. + */ + using DOFsPtr = decltype(std::declval().getDOFsObject().get()); + DOFsPtr m_dofs = nullptr; + bool m_pc_diag_only = false; int m_pc_mat_nnz = 1; bool m_include_mass_matrices = false; @@ -247,9 +258,6 @@ void MatrixPC::Define ( const T& a_U, // read preconditioner parameters readParameters(); - // a_U is not needed - amrex::ignore_unused(a_U); - // Set number of AMR levels and create geometry, grids, and // distribution mapping vectors. m_num_amr_levels = m_ops->numAMRLevels(); @@ -261,6 +269,13 @@ void MatrixPC::Define ( const T& a_U, m_geom[n] = m_ops->GetGeometry(n); } + // Cache the DOF numbering object, so that Assemble() can read the local + // and global DOF index maps without being handed a vector. + m_dofs = a_U.getDOFsObject().get(); + WARPX_ALWAYS_ASSERT_WITH_MESSAGE( + (m_dofs != nullptr), + "MatrixPC::Define(): the solver vector has no DOF object" ); + m_ndofs_l = a_U.nDOF_local(); m_ndofs_g = a_U.nDOF_global(); @@ -282,7 +297,7 @@ void MatrixPC::Define ( const T& a_U, } template -void MatrixPC::Update (const T& a_U) +void MatrixPC::Update () { BL_PROFILE("MatrixPC::Update()"); using namespace amrex; @@ -293,7 +308,7 @@ void MatrixPC::Update (const T& a_U) while(true) { - auto nnz_diff = Assemble(a_U); + auto nnz_diff = Assemble(); AMREX_ALWAYS_ASSERT(nnz_diff >= 0); if (nnz_diff) { @@ -315,7 +330,7 @@ void MatrixPC::Update (const T& a_U) } template -int MatrixPC::Assemble (const T& a_U) +int MatrixPC::Assemble () { // Assemble the sparse matrix representation of the preconditioner // A = curl (alpha * curl []) + M @@ -347,11 +362,10 @@ int MatrixPC::Assemble (const T& a_U) << "alpha = " << alpha << "\n"; } - // Get DOF object from a_U - const auto& dofs_obj = a_U.getDOFsObject(); - const auto& dofs_mfarrvec = dofs_obj->m_array; - AMREX_ALWAYS_ASSERT(m_ndofs_l == dofs_obj->m_nDoFs_l); - AMREX_ALWAYS_ASSERT(m_ndofs_g == dofs_obj->m_nDoFs_g); + // DOF object cached in Define() + const auto& dofs_mfarrvec = m_dofs->m_array; + AMREX_ALWAYS_ASSERT(m_ndofs_l == m_dofs->m_nDoFs_l); + AMREX_ALWAYS_ASSERT(m_ndofs_g == m_dofs->m_nDoFs_g); m_r_indices_g.clear(); m_num_nz.clear(); diff --git a/Source/NonlinearSolvers/NewtonSolver.H b/Source/NonlinearSolvers/NewtonSolver.H index 4debb5c9429..1ed7b40afa2 100644 --- a/Source/NonlinearSolvers/NewtonSolver.H +++ b/Source/NonlinearSolvers/NewtonSolver.H @@ -388,7 +388,7 @@ void NewtonSolver::Solve (Vec& a_U, m_ops->PreLinearSolve(); m_linear_function->setBaseSolution(a_U); m_linear_function->setBaseRHS(m_R); - m_linear_function->updatePreCondMat(a_U); + m_linear_function->updatePreCondMat(); // Solve linear system for Newton step [Jac]*dU = F m_dU.zero(); diff --git a/Source/NonlinearSolvers/Preconditioner.H b/Source/NonlinearSolvers/Preconditioner.H index 1a2ec025bcb..41ea1384d1b 100644 --- a/Source/NonlinearSolvers/Preconditioner.H +++ b/Source/NonlinearSolvers/Preconditioner.H @@ -57,8 +57,14 @@ class Preconditioner /** * \brief Update the preconditioner + * + * This takes no argument: preconditioners draw the state they need + * from the Ops object and the grid layout they cached in Define(), + * plus the time and time step size set via CurTime()/CurTimeStep(). + * Should a preconditioner ever need to re-linearize about the + * current iterate, pass it in here. */ - virtual void Update ( const T& a_U ) = 0; + virtual void Update () = 0; /** * \brief Apply (solve) the preconditioner given a RHS diff --git a/Source/NonlinearSolvers/WarpX_PETSc.cpp b/Source/NonlinearSolvers/WarpX_PETSc.cpp index 124649bf6c0..1fb3b08a610 100644 --- a/Source/NonlinearSolvers/WarpX_PETSc.cpp +++ b/Source/NonlinearSolvers/WarpX_PETSc.cpp @@ -106,7 +106,7 @@ PetscErrorCode RHSFunction( SNES a_solver, Vec a_U, Vec a_F, void* ctxt) VecAXPBY(a_F, 1.0, -1.0, a_U); if (!context->m_fd_jac_comput) { - dynamic_cast*>(context->m_linop.get())->updatePreCondMat(context->m_U); + dynamic_cast*>(context->m_linop.get())->updatePreCondMat(); } PetscFunctionReturn(PETSC_SUCCESS); } From d80b0f866ac95b5ba056fb2c3c071f2513feabe9 Mon Sep 17 00:00:00 2001 From: Remi Lehe Date: Wed, 2 Sep 2026 14:02:02 -0700 Subject: [PATCH 2/9] Apply suggestion from @RemiLehe --- Source/NonlinearSolvers/JacobiPC.H | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Source/NonlinearSolvers/JacobiPC.H b/Source/NonlinearSolvers/JacobiPC.H index 2ccfae3bddb..d1b177b2d0e 100644 --- a/Source/NonlinearSolvers/JacobiPC.H +++ b/Source/NonlinearSolvers/JacobiPC.H @@ -276,9 +276,8 @@ void JacobiPC::Define ( const T& a_U, m_num_amr_levels = m_ops->numAMRLevels(); m_bcoefs = m_ops->GetMassMatricesCoeff(); - // Cache the grid layout of the solver vector. The scratch MultiFabs are - // allocated lazily in Update(), once the mass-matrix coefficients are - // known, but their layout is already fixed here. + // Save the grid layout of the solver vector. This is then used in + // `Update` to allocate temporary MultiFabs const auto& u_mfarrvec = a_U.getArrayVec(); m_grids.resize(m_num_amr_levels); m_dmap.resize(m_num_amr_levels); From 23092a53ae3c346ecd193eb0a1d1cb5f10b0a80e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:02:12 +0000 Subject: [PATCH 3/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- Source/NonlinearSolvers/JacobiPC.H | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/NonlinearSolvers/JacobiPC.H b/Source/NonlinearSolvers/JacobiPC.H index d1b177b2d0e..b934ea8cda9 100644 --- a/Source/NonlinearSolvers/JacobiPC.H +++ b/Source/NonlinearSolvers/JacobiPC.H @@ -276,7 +276,7 @@ void JacobiPC::Define ( const T& a_U, m_num_amr_levels = m_ops->numAMRLevels(); m_bcoefs = m_ops->GetMassMatricesCoeff(); - // Save the grid layout of the solver vector. This is then used in + // Save the grid layout of the solver vector. This is then used in // `Update` to allocate temporary MultiFabs const auto& u_mfarrvec = a_U.getArrayVec(); m_grids.resize(m_num_amr_levels); From 8545f27074876f4b7d91feafc98f58f400bc7f17 Mon Sep 17 00:00:00 2001 From: Remi Lehe Date: Wed, 2 Sep 2026 14:02:50 -0700 Subject: [PATCH 4/9] Cache only the DOF index maps in MatrixPC The `decltype(std::declval().getDOFsObject().get())` alias was over-engineered: `MatrixPC` already hard-codes `FieldType::Efield_fp`, so it is not generic, and the alias was hard to read for no benefit. `Assemble()` only ever needed the DOF index maps, so cache those directly. Their type is expressible in plain AMReX terms, which removes the alias without naming `WarpXSolverDOF` or adding a NonlinearSolvers -> ImplicitSolvers include. Also drop the two DOF-count asserts. `WarpXSolverVec` owns a single static DOF object shared by every solver vector, so these compared `m_ndofs_*` against the very storage they had been copied from in `Define()` and could not fail. The separate null check is gone too: the `nDOF_local()`/`nDOF_global()` queries just above already assert that the DOF object exists. Co-Authored-By: Claude Opus 5 --- Source/NonlinearSolvers/MatrixPC.H | 32 ++++++++++++++---------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/Source/NonlinearSolvers/MatrixPC.H b/Source/NonlinearSolvers/MatrixPC.H index a73897bbf01..0f5788e4a0d 100644 --- a/Source/NonlinearSolvers/MatrixPC.H +++ b/Source/NonlinearSolvers/MatrixPC.H @@ -20,8 +20,10 @@ #include #include #include +#include -#include +#include +#include namespace MatrixPCUtils { @@ -189,12 +191,12 @@ class MatrixPC : public Preconditioner int m_ndofs_g = 0; /** - * \brief The DOF numbering object of the solver vector, cached in - * Define(). The solver vector class owns a single instance of it that - * lives until AMReX finalization, so a non-owning pointer is safe. + * \brief The per-level, per-direction DOF index maps of the solver + * vector, cached in Define(). The solver vector class owns a single + * DOF object that lives until AMReX finalization, so a non-owning + * pointer is safe. */ - using DOFsPtr = decltype(std::declval().getDOFsObject().get()); - DOFsPtr m_dofs = nullptr; + const amrex::Vector,3>>* m_dofs_array = nullptr; bool m_pc_diag_only = false; int m_pc_mat_nnz = 1; @@ -269,16 +271,14 @@ void MatrixPC::Define ( const T& a_U, m_geom[n] = m_ops->GetGeometry(n); } - // Cache the DOF numbering object, so that Assemble() can read the local - // and global DOF index maps without being handed a vector. - m_dofs = a_U.getDOFsObject().get(); - WARPX_ALWAYS_ASSERT_WITH_MESSAGE( - (m_dofs != nullptr), - "MatrixPC::Define(): the solver vector has no DOF object" ); - m_ndofs_l = a_U.nDOF_local(); m_ndofs_g = a_U.nDOF_global(); + // Cache the DOF index maps, so that Assemble() can read them without being + // handed a vector. The nDOF queries above already assert that the solver + // vector's DOF object exists. + m_dofs_array = &a_U.getDOFsObject()->m_array; + auto n_rows = size_t(m_ndofs_l); auto n_cols = size_t(m_pc_mat_nnz) * size_t(m_ndofs_l); @@ -362,10 +362,8 @@ int MatrixPC::Assemble () << "alpha = " << alpha << "\n"; } - // DOF object cached in Define() - const auto& dofs_mfarrvec = m_dofs->m_array; - AMREX_ALWAYS_ASSERT(m_ndofs_l == m_dofs->m_nDoFs_l); - AMREX_ALWAYS_ASSERT(m_ndofs_g == m_dofs->m_nDoFs_g); + // DOF index maps cached in Define() + const auto& dofs_mfarrvec = *m_dofs_array; m_r_indices_g.clear(); m_num_nz.clear(); From 0a7d1f94b6a0620029e6a768c7277554123789d2 Mon Sep 17 00:00:00 2001 From: Remi Lehe Date: Wed, 2 Sep 2026 15:21:45 -0700 Subject: [PATCH 5/9] Apply suggestion from @RemiLehe --- Source/NonlinearSolvers/Preconditioner.H | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Source/NonlinearSolvers/Preconditioner.H b/Source/NonlinearSolvers/Preconditioner.H index 41ea1384d1b..0822744be05 100644 --- a/Source/NonlinearSolvers/Preconditioner.H +++ b/Source/NonlinearSolvers/Preconditioner.H @@ -57,12 +57,6 @@ class Preconditioner /** * \brief Update the preconditioner - * - * This takes no argument: preconditioners draw the state they need - * from the Ops object and the grid layout they cached in Define(), - * plus the time and time step size set via CurTime()/CurTimeStep(). - * Should a preconditioner ever need to re-linearize about the - * current iterate, pass it in here. */ virtual void Update () = 0; From 10f277cd3d54f9aeabe168782e789cc5a220cf39 Mon Sep 17 00:00:00 2001 From: Remi Lehe Date: Wed, 2 Sep 2026 15:24:48 -0700 Subject: [PATCH 6/9] Reach the DOF object statically instead of storing it in MatrixPC `WarpXSolverVec` owns a single static DOF object shared by every solver vector, so `Assemble()` never needed an instance to reach it, and nothing needed caching in the first place. Make `getDOFsObject()` static -- it already returned static state, so the `const` member form was misleading -- and read the index maps through it directly. This removes the cached member, the `decltype(std::declval<...>())` alias that spelled its type, the caching block in `Define()`, its null-check assert, and the `` include. `MatrixPC.H` now differs from before this PR only in the `Update()`/`Assemble()` signatures and this one line. Co-Authored-By: Claude Opus 5 --- .../ImplicitSolvers/WarpXSolverVec.H | 5 +++-- Source/NonlinearSolvers/MatrixPC.H | 22 +++---------------- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H b/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H index 62a4329d616..3c7fd4bc663 100644 --- a/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H +++ b/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H @@ -313,8 +313,9 @@ public: // return the number of AMR levels [[nodiscard]] auto numAMRLevels () const { return m_num_amr_levels; } - // return DOFs object pointer - [[nodiscard]] inline const auto& getDOFsObject () const { return m_dofs; } + // return DOFs object pointer. Static, since the DOF object is shared by + // every solver vector, so callers do not need an instance to reach it. + [[nodiscard]] inline static const auto& getDOFsObject () { return m_dofs; } private: diff --git a/Source/NonlinearSolvers/MatrixPC.H b/Source/NonlinearSolvers/MatrixPC.H index 0f5788e4a0d..5fd840de3b0 100644 --- a/Source/NonlinearSolvers/MatrixPC.H +++ b/Source/NonlinearSolvers/MatrixPC.H @@ -20,10 +20,6 @@ #include #include #include -#include - -#include -#include namespace MatrixPCUtils { @@ -190,14 +186,6 @@ class MatrixPC : public Preconditioner int m_ndofs_l = 0; int m_ndofs_g = 0; - /** - * \brief The per-level, per-direction DOF index maps of the solver - * vector, cached in Define(). The solver vector class owns a single - * DOF object that lives until AMReX finalization, so a non-owning - * pointer is safe. - */ - const amrex::Vector,3>>* m_dofs_array = nullptr; - bool m_pc_diag_only = false; int m_pc_mat_nnz = 1; bool m_include_mass_matrices = false; @@ -274,11 +262,6 @@ void MatrixPC::Define ( const T& a_U, m_ndofs_l = a_U.nDOF_local(); m_ndofs_g = a_U.nDOF_global(); - // Cache the DOF index maps, so that Assemble() can read them without being - // handed a vector. The nDOF queries above already assert that the solver - // vector's DOF object exists. - m_dofs_array = &a_U.getDOFsObject()->m_array; - auto n_rows = size_t(m_ndofs_l); auto n_cols = size_t(m_pc_mat_nnz) * size_t(m_ndofs_l); @@ -362,8 +345,9 @@ int MatrixPC::Assemble () << "alpha = " << alpha << "\n"; } - // DOF index maps cached in Define() - const auto& dofs_mfarrvec = *m_dofs_array; + // The DOF object is shared by every solver vector, so it can be reached + // without one. + const auto& dofs_mfarrvec = T::getDOFsObject()->m_array; m_r_indices_g.clear(); m_num_nz.clear(); From afed7261f56450b7c92b4aae062d32cecefac3ff Mon Sep 17 00:00:00 2001 From: Remi Lehe Date: Wed, 2 Sep 2026 15:28:45 -0700 Subject: [PATCH 7/9] Apply batched suggestions from code review Co-authored-by: Remi Lehe --- Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H | 2 +- Source/NonlinearSolvers/MatrixPC.H | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H b/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H index 3c7fd4bc663..9604adfa6f5 100644 --- a/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H +++ b/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H @@ -314,7 +314,7 @@ public: [[nodiscard]] auto numAMRLevels () const { return m_num_amr_levels; } // return DOFs object pointer. Static, since the DOF object is shared by - // every solver vector, so callers do not need an instance to reach it. + // every solver vector. [[nodiscard]] inline static const auto& getDOFsObject () { return m_dofs; } private: diff --git a/Source/NonlinearSolvers/MatrixPC.H b/Source/NonlinearSolvers/MatrixPC.H index 5fd840de3b0..573a4a32368 100644 --- a/Source/NonlinearSolvers/MatrixPC.H +++ b/Source/NonlinearSolvers/MatrixPC.H @@ -345,8 +345,7 @@ int MatrixPC::Assemble () << "alpha = " << alpha << "\n"; } - // The DOF object is shared by every solver vector, so it can be reached - // without one. + // The DOF object is shared by every solver vector. const auto& dofs_mfarrvec = T::getDOFsObject()->m_array; m_r_indices_g.clear(); From cbaedcb96c1fe5f435f1a8a2401c970f72e6d661 Mon Sep 17 00:00:00 2001 From: Remi Lehe Date: Wed, 2 Sep 2026 15:37:03 -0700 Subject: [PATCH 8/9] Apply suggestion from @RemiLehe --- Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H b/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H index 9604adfa6f5..968cab6cf93 100644 --- a/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H +++ b/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H @@ -313,8 +313,7 @@ public: // return the number of AMR levels [[nodiscard]] auto numAMRLevels () const { return m_num_amr_levels; } - // return DOFs object pointer. Static, since the DOF object is shared by - // every solver vector. + // return DOFs object pointer. [[nodiscard]] inline static const auto& getDOFsObject () { return m_dofs; } private: From 1fd23b358bbc2eefd4afcacc0aa6c8da01299c71 Mon Sep 17 00:00:00 2001 From: Remi Lehe Date: Wed, 2 Sep 2026 15:37:19 -0700 Subject: [PATCH 9/9] Apply suggestion from @RemiLehe --- Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H b/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H index 968cab6cf93..8300a2eb5ed 100644 --- a/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H +++ b/Source/FieldSolver/ImplicitSolvers/WarpXSolverVec.H @@ -313,7 +313,7 @@ public: // return the number of AMR levels [[nodiscard]] auto numAMRLevels () const { return m_num_amr_levels; } - // return DOFs object pointer. + // return DOFs object pointer [[nodiscard]] inline static const auto& getDOFsObject () { return m_dofs; } private: