Skip to content

Nodal projection in/out solvability rescales m_leveldata velocity instead of the vel argument #184

Description

@WeiqunZhang

Severity: medium · Category: correctness · Fix order: 12 of 29 — fix this 12th.

Filenames are numbered in reverse fix order: 001 = fix last, 029 = fix first. This file is 018.

Location: src/projection/incflo_apply_nodal_projection.cpp:171

Based on commit 7307d872 (line numbers refer to that tree).

The defect

What is wrong — The in/out solvability block aliases m_leveldata[lev]->velocity instead of the vel[lev] parameter, so when the low-level ApplyNodalProjection is called with a different velocity vector, solvability is enforced on the wrong field and the state velocity is modified.

Why it matters

Nodal projection + a direction_dependent BC (has_inout_bndry=true) + incflo.do_initial_pressure_proj=1: InitialPressureProjection (init.cpp:610) passes a local gravity-based vel; enforceInOutSolvability rescales ld.velocity's boundary cells (corrupting the state) while the projected field keeps its influx/outflux imbalance, giving a non-solvable nodal solve and wrong hydrostatic p/gp.

Suggested fix

In the low-level ApplyNodalProjection overload, build the enforceInOutSolvability aliases from the vel parameter rather than m_leveldata[lev]->velocity. HydroUtils::enforceInOutSolvability mutates the passed field in place (it rescales outflow boundary values so total outflux matches influx), and the nodal projector at lines 185/189 consumes vel — so the field being projected must be the one rebalanced, exactly as the MAC twin does by passing mac_vec (the same object later given to macproj->setUMAC) in incflo_compute_MAC_projected_velocities.cpp. Aliasing vel is consistent with the rest of the function, which already operates on vel[lev] for setBndry and the inflow physbc fill. In the normal timestep path vel aliases the state velocity, so behavior there is unchanged; the maintainer should just confirm no caller passing a separate vector (e.g. InitialPressureProjection) relied on the old side effect of the state velocity being rescaled.

--- a/src/projection/incflo_apply_nodal_projection.cpp
+++ b/src/projection/incflo_apply_nodal_projection.cpp
@@ -167,10 +167,9 @@
         Vector<Array<MultiFab, AMREX_SPACEDIM>> vel_vec(finest_level+1);
 
         for (int lev = 0; lev <= finest_level; lev++) {
-            auto& ld = *m_leveldata[lev];
-            AMREX_D_TERM(vel_vec[lev][0] = MultiFab(ld.velocity, amrex::make_alias, 0, 1);,
-                         vel_vec[lev][1] = MultiFab(ld.velocity, amrex::make_alias, 1, 1);,
-                         vel_vec[lev][2] = MultiFab(ld.velocity, amrex::make_alias, 2, 1););
+            AMREX_D_TERM(vel_vec[lev][0] = MultiFab(*vel[lev], amrex::make_alias, 0, 1);,
+                         vel_vec[lev][1] = MultiFab(*vel[lev], amrex::make_alias, 1, 1);,
+                         vel_vec[lev][2] = MultiFab(*vel[lev], amrex::make_alias, 2, 1););
         }
 
         HydroUtils::enforceInOutSolvability(GetVecOfArrOfPtrs(vel_vec), get_velocity_bcrec().data(), geom, true);

Aliases the vel parameter (the field actually projected at lines 185/189) instead of m_leveldata state, matching the MAC twin which passes mac_vec. HydroUtils::enforceInOutSolvability (AMReX-Hydro Utils/hydro_utils.H) rescales the passed MultiFabs in place. The now-unused 'auto& ld' local is removed to avoid an unused-variable warning. Behavior unchanged for the main overload where vel[lev] == &ld.velocity.

Diff(s) are against 7307d872, written from the current source and verified only with git apply --check — never compiled, never run, never applied to the tree. Treat them as precise intent, not tested patches.

Verification evidence

F038 — confirmed (one verifier lens)

Lens 1 (refutation attempt): Lines 171-173: vel_vec[lev][0] = MultiFab(ld.velocity, amrex::make_alias, 0, 1) inside the overload whose parameter is vel; line 176 passes these aliases to enforceInOutSolvability, while line 185/189 projects vel. init.cpp:610 calls this overload with a local gravity-based vel and set_inflow_bc=true; has_inout_bndry is true for direction_dependent BCs. AMReX-Hydro's correct_outflow rescales the passed field in place, so the state velocity is mutated and the projected field keeps its imbalance. The MAC twin (line 269 of incflo_compute_MAC_projected_velocities.cpp) correctly uses mac_vec. No fix in git history.


Based on commit 7307d872, which is also the tree the audit verified against. From an automated audit of src/. Audit finding id: F038. Reviewer unit(s): Projection. Nothing here was compiled or run — the failure scenarios are code reasoning, so the reaching configuration above is the cheapest way to confirm or refute it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions