From d8979e9b3ab618c870c0e453709401451310c911 Mon Sep 17 00:00:00 2001 From: Ann Almgren Date: Wed, 7 Oct 2026 20:32:21 -0700 Subject: [PATCH 1/3] Fix issues #222, #224, #226, #230 #222 main(): a run setting only stop_interval aborted before stop_time was derived from it. Count stop_interval as a valid stopping criterion in the guard, and name all three in the message. #224 velocity_update()/scalar_update(): a NaN in the new state ended the run with exit(0), reporting success to batch scripts and CI. Use amrex::Abort so the run stops with a nonzero status, a backtrace and a proper Finalize. #226 ComputeAofs(): the non-EB flux-register path ignored do_crse_add and do_fine_add, so the mac_sync added coarse fluxes that the EB twin skips. Honour both flags as the EB branch does; do_fine_add is now used in every build config, so drop its ignore_unused. #230 Initialize_bcs(): an old-style ns.lo_bc/hi_bc entry silently overrode a conflicting xlo.type, defeating the "Multiple conflicting BCs" check. Let the integer table fill in only when no string type was given, so a mismatch reaches that check. Also read lo_bc and hi_bc as a pair -- ns.hi_bc alone was previously parsed into nothing and dropped. Verified: regtest.2d.poiseuille (amr.max_level=1, exercising the #226 mac_sync path) is bit-identical to development; the 2D non-EB and EB regression decks run clean apart from eb_run2d shock_past_cylinder, which fails identically on development. Co-Authored-By: Claude Opus 5 (1M context) --- Source/NavierStokes.cpp | 21 +++++++++++++++------ Source/NavierStokesBase.cpp | 14 ++++++-------- Source/main.cpp | 4 ++-- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/Source/NavierStokes.cpp b/Source/NavierStokes.cpp index 218028d6..49bdc576 100644 --- a/Source/NavierStokes.cpp +++ b/Source/NavierStokes.cpp @@ -96,8 +96,17 @@ NavierStokes::Initialize_bcs () // // Check for integer BC type specification in inputs file (older style) // - if ( pp.contains("lo_bc") ) + if ( pp.contains("lo_bc") || pp.contains("hi_bc") ) { + // + // The two are a pair: reading hi_bc only when lo_bc is present would + // silently drop a deck that gives just one of them. + // + if ( !pp.contains("lo_bc") || !pp.contains("hi_bc") ) + { + amrex::Abort("NavierStokes::Initialize_bcs: ns.lo_bc and ns.hi_bc must be given together."); + } + Vector lo_bc(AMREX_SPACEDIM), hi_bc(AMREX_SPACEDIM); pp.getarr("lo_bc",lo_bc,0,AMREX_SPACEDIM); pp.getarr("hi_bc",hi_bc,0,AMREX_SPACEDIM); @@ -134,7 +143,7 @@ NavierStokes::Initialize_bcs () std::string bc_type = amrex::toLower(bc_type_in); if (bc_type == "no_slip_wall" or bc_type == "nsw" - or phys_bc.data()[ori] == PhysBCType::noslipwall) + or (bc_type == "null" and phys_bc.data()[ori] == PhysBCType::noslipwall)) { amrex::Print() << bcid <<" set to no-slip wall.\n"; @@ -165,7 +174,7 @@ NavierStokes::Initialize_bcs () // m_bc_velocity[ori] = {0.0, 0.0, 0.0}; } else if (bc_type == "mass_inflow" or bc_type == "mi" - or phys_bc.data()[ori] == PhysBCType::inflow) + or (bc_type == "null" and phys_bc.data()[ori] == PhysBCType::inflow)) { amrex::Print() << bcid << " set to mass inflow.\n"; @@ -199,7 +208,7 @@ NavierStokes::Initialize_bcs () // pbc.get("pressure", m_bc_pressure[ori]); } else if (bc_type == "pressure_outflow" or bc_type == "po" - or phys_bc.data()[ori] == PhysBCType::outflow) + or (bc_type == "null" and phys_bc.data()[ori] == PhysBCType::outflow)) { amrex::Print() << bcid << " set to pressure outflow.\n"; @@ -869,8 +878,8 @@ NavierStokes::scalar_update (Real dt, { if (S_new.contains_nan(sigma,1,0)) { - Print() << "New scalar " << sigma << " contains Nans" << '\n'; - exit(0); + amrex::Abort("NavierStokes::scalar_update(): new scalar component " + + std::to_string(sigma) + " contains NaNs"); } } } diff --git a/Source/NavierStokesBase.cpp b/Source/NavierStokesBase.cpp index 3107f2c5..1f61f6bb 100644 --- a/Source/NavierStokesBase.cpp +++ b/Source/NavierStokesBase.cpp @@ -3572,8 +3572,8 @@ NavierStokesBase::velocity_update (Real dt) { if (S_new.contains_nan(sigma,1,0)) { - amrex::Print() << "New velocity " << sigma << " contains Nans" << '\n'; - exit(0); + amrex::Abort("NavierStokesBase::velocity_update(): new velocity component " + + std::to_string(sigma) + " contains NaNs"); } } } @@ -4721,8 +4721,6 @@ NavierStokesBase::ComputeAofs ( MultiFab& advc, int a_comp, // Advection term "A { BL_PROFILE("NSB::ComputeAofs_kernel"); - amrex::ignore_unused(do_fine_add); - // Need U_corr to be defined for sync. AMREX_ASSERT( (is_sync && !U_corr.empty()) || !is_sync ); @@ -5270,17 +5268,17 @@ NavierStokesBase::ComputeAofs ( MultiFab& advc, int a_comp, // Advection term "A Real sync_factor = do_crse_add ? 1.0 : -1.0; // Update the flux registers when no EB - if ( do_reflux && (level < parent->finestLevel()) ) { + if ( do_reflux && do_crse_add && (level < parent->finestLevel()) ) { getAdvFluxReg(level+1).CrseAdd(mfi, {AMREX_D_DECL(&fx_fab,&fy_fab,&fz_fab)}, dxDp, sync_factor*dt, flux_comp, state_indx, ncomp, amrex::RunOn::Device); - } // do_reflux && level < finest_level + } // do_reflux && do_crse_add && level < finest_level - if ( do_reflux && (level > 0) ) { + if ( do_reflux && do_fine_add && (level > 0) ) { advflux_reg->FineAdd(mfi, {AMREX_D_DECL(&fx_fab,&fy_fab,&fz_fab)}, dxDp, sync_factor*dt, flux_comp, state_indx, ncomp, amrex::RunOn::Device); - } // do_reflux && (level > 0) + } // do_reflux && do_fine_add && (level > 0) #endif } // mfi } diff --git a/Source/main.cpp b/Source/main.cpp index bcd5ae6b..7c1c115a 100644 --- a/Source/main.cpp +++ b/Source/main.cpp @@ -59,9 +59,9 @@ main (int argc, amrex::Abort("MUST SPECIFY a non-negative strt_time"); } - if (max_step < 0 && stop_time < 0) + if (max_step < 0 && stop_time < 0 && stop_interval <= 0.) { - amrex::Abort("Exiting because neither max_step nor stop_time is non-negative."); + amrex::Abort("Exiting because none of max_step, stop_time or stop_interval is set."); } Amr* amrptr = new Amr(getLevelBld()); From 3fd14d64ff614d81ba6e020591d0921cd057666b Mon Sep 17 00:00:00 2001 From: Ann Almgren Date: Wed, 7 Oct 2026 20:32:29 -0700 Subject: [PATCH 2/3] init_ConvectedVortex: put the meanFlowDir = +/-3 mean flow on z case 3 added meanFlowMag to both the x and y velocity components and left z untouched; case -3 did the same with the opposite sign. Set the z component only, matching the single-component pattern of case +/-1 and +/-2. With the mean flow correctly on z, AMREX_D_TERM drops it entirely in 2D, so meanFlowDir = +/-3 would silently run as "no mean flow". Extend the validation guard above to abort in that case instead. Verified in 3D with prob.meanFlowMag = 50, reported as max|u| / max|v| / max|w|: dir= 0 -> 51.6 / 51.6 / 0 (vortex alone) dir= 1 -> 101.6 / 51.6 / 0 dir= 2 -> 51.6 / 101.6 / 0 dir= 3 -> 51.6 / 51.6 / 50 Before this change dir=3 gave 101.6 / 101.6 / 0. Co-Authored-By: Claude Opus 5 (1M context) --- Source/prob/prob_init.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Source/prob/prob_init.cpp b/Source/prob/prob_init.cpp index 58bb6ff7..b3a88ef0 100644 --- a/Source/prob/prob_init.cpp +++ b/Source/prob/prob_init.cpp @@ -649,6 +649,11 @@ void NavierStokes::init_ConvectedVortex (Box const& vbx, if ( IC.meanFlowDir < -3 || IC.meanFlowDir > 3 ) { amrex::Abort("\n init_ConvectedVortex: prob.meanFlowDir must be 0 (no mean flow) or +/-1, +/-2, +/-3\n in the inputs file."); } +#if (AMREX_SPACEDIM == 2) + if ( IC.meanFlowDir == 3 || IC.meanFlowDir == -3 ) { + amrex::Abort("\n init_ConvectedVortex: prob.meanFlowDir = +/-3 requires 3D\n in the inputs file."); + } +#endif amrex::ParallelFor(vbx, [=] AMREX_GPU_DEVICE (int i, int j, int k) noexcept { @@ -691,14 +696,14 @@ void NavierStokes::init_ConvectedVortex (Box const& vbx, vel(i,j,k,2) = w_vort); break; case 3 : - AMREX_D_TERM(vel(i,j,k,0) = IC.meanFlowMag + u_vort;, - vel(i,j,k,1) = IC.meanFlowMag + v_vort;, - vel(i,j,k,2) = w_vort); + AMREX_D_TERM(vel(i,j,k,0) = u_vort;, + vel(i,j,k,1) = v_vort;, + vel(i,j,k,2) = IC.meanFlowMag + w_vort); break; case -3 : - AMREX_D_TERM(vel(i,j,k,0) = -IC.meanFlowMag + u_vort;, - vel(i,j,k,1) = -IC.meanFlowMag + v_vort;, - vel(i,j,k,2) = w_vort); + AMREX_D_TERM(vel(i,j,k,0) = u_vort;, + vel(i,j,k,1) = v_vort;, + vel(i,j,k,2) = -IC.meanFlowMag + w_vort); break; case 0 : // no mean flow, i.e. the vortex alone default : From ff09d46a29617a0f5c85fe35b3a7524ef4835c7a Mon Sep 17 00:00:00 2001 From: Ann Almgren Date: Thu, 8 Oct 2026 09:34:41 -0700 Subject: [PATCH 3/3] Address review: keep the diagonal mean flow as meanFlowDir = +/-4 Per @cgilet's review of #258: the original meanFlowDir = +/-3, which added meanFlowMag to both the x and y velocity components, was not a bug. It was a deliberate non-axis-aligned mean flow for a problem that is inherently 2D but may be run as pseudo-3D. The previous commit removed that capability. Restore it under a new number rather than dropping it: 0 no mean flow (the vortex alone) +/-1 mean flow along x +/-2 mean flow along y +/-3 mean flow along z (3D only) +/-4 diagonal mean flow in the x-y plane case +/-4 is the pre-PR case +/-3 body verbatim, so a deck that used 3 reproduces its old results exactly by switching to 4. Note the diagonal has |U| = meanFlowMag*sqrt(2), as it always did. The 2D guard is kept but now applies only to +/-3, where AMREX_D_TERM drops the z component and the mean flow would silently vanish; its message points at +/-4, which is what a 2D user wanting a diagonal flow needs. +/-4 itself works in 2D. Also document the encoding in ProblemSetup.rst, which previously listed no valid values for meanFlowDir at all. Verified in 3D with prob.meanFlowMag = 50, as max|u| / max|v| / max|w|: dir= 0 -> 51.6 / 51.6 / 0 dir= 1 -> 101.6 / 51.6 / 0 dir= 2 -> 51.6 / 101.6 / 0 dir= 3 -> 51.6 / 51.6 / 50 dir= 4 -> 101.6 / 101.6 / 0 (matches pre-PR dir=3) Co-Authored-By: Claude Opus 5 (1M context) --- .../source/ProblemSetup.rst | 4 +++- Source/prob/prob_init.cpp | 24 ++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/Docs/sphinx_documentation/source/ProblemSetup.rst b/Docs/sphinx_documentation/source/ProblemSetup.rst index b52a4c01..3e84085f 100644 --- a/Docs/sphinx_documentation/source/ProblemSetup.rst +++ b/Docs/sphinx_documentation/source/ProblemSetup.rst @@ -106,7 +106,9 @@ Which of them a given ``probtype`` actually reads varies; see ``prob_init.cpp``. +------------------------+--------------------------------------------------------------------+-------------+-----------+ | forcevort | Strength of the convected vortex (probtype 8) | Real | 6.0 | +------------------------+--------------------------------------------------------------------+-------------+-----------+ -| meanFlowDir | Direction of the mean flow the vortex is convected by (probtype 8) | Int | 0 | +| meanFlowDir | Direction of the mean flow the vortex is convected by (probtype 8).| Int | 0 | +| | 0 = no mean flow; +/-1, +/-2, +/-3 = mean flow along x, y, z; | | | +| | +/-4 = diagonal mean flow in the x-y plane. +/-3 requires 3D. | | | +------------------------+--------------------------------------------------------------------+-------------+-----------+ | meanFlowMag | Magnitude of that mean flow (probtype 8) | Real | 0.0 | +------------------------+--------------------------------------------------------------------+-------------+-----------+ diff --git a/Source/prob/prob_init.cpp b/Source/prob/prob_init.cpp index b3a88ef0..23e6fdb7 100644 --- a/Source/prob/prob_init.cpp +++ b/Source/prob/prob_init.cpp @@ -646,12 +646,20 @@ void NavierStokes::init_ConvectedVortex (Box const& vbx, // amrex::Abort() cannot be called from within the device lambda below, so // validate the mean flow direction here. // - if ( IC.meanFlowDir < -3 || IC.meanFlowDir > 3 ) { - amrex::Abort("\n init_ConvectedVortex: prob.meanFlowDir must be 0 (no mean flow) or +/-1, +/-2, +/-3\n in the inputs file."); + if ( IC.meanFlowDir < -4 || IC.meanFlowDir > 4 ) { + amrex::Abort("\n init_ConvectedVortex: prob.meanFlowDir must be 0 (no mean flow)," + "\n +/-1, +/-2, +/-3 (mean flow along x, y, z) or +/-4 (diagonal in the x-y plane)" + "\n in the inputs file."); } #if (AMREX_SPACEDIM == 2) + // + // +/-4 is the non-axis-aligned option and works in 2D; +/-3 does not, + // since AMREX_D_TERM drops the z component and the mean flow would + // silently vanish. + // if ( IC.meanFlowDir == 3 || IC.meanFlowDir == -3 ) { - amrex::Abort("\n init_ConvectedVortex: prob.meanFlowDir = +/-3 requires 3D\n in the inputs file."); + amrex::Abort("\n init_ConvectedVortex: prob.meanFlowDir = +/-3 puts the mean flow along z" + "\n and requires 3D. Use +/-4 for a diagonal mean flow in the x-y plane."); } #endif @@ -705,6 +713,16 @@ void NavierStokes::init_ConvectedVortex (Box const& vbx, vel(i,j,k,1) = v_vort;, vel(i,j,k,2) = -IC.meanFlowMag + w_vort); break; + case 4 : // diagonal in the x-y plane; |U| = meanFlowMag*sqrt(2) + AMREX_D_TERM(vel(i,j,k,0) = IC.meanFlowMag + u_vort;, + vel(i,j,k,1) = IC.meanFlowMag + v_vort;, + vel(i,j,k,2) = w_vort); + break; + case -4 : + AMREX_D_TERM(vel(i,j,k,0) = -IC.meanFlowMag + u_vort;, + vel(i,j,k,1) = -IC.meanFlowMag + v_vort;, + vel(i,j,k,2) = w_vort); + break; case 0 : // no mean flow, i.e. the vortex alone default : AMREX_D_TERM(vel(i,j,k,0) = u_vort;,