Fix issues #222, #224, #226, #230; fix meanFlowDir = +/-3 in init_ConvectedVortex

[asalmgren]

Fixes #222,
fixes #224,
fixes #226,
fixes #230,
plus one bug found while reviewing already-merged code.

Issues addressed

#222 — main(): a run that sets only stop_interval aborts before stop_time is derived from it. Count stop_interval as a valid stopping criterion in the guard, and name all three in the message.

#224 — a NaN in the new state ends the run with exit(0), reporting success. Both sites (velocity_update, scalar_update) now use amrex::Abort, so the run stops with a nonzero status, a backtrace and a proper Finalize. No exit(0)/exit(1) remains anywhere in Source/.

#226 — ComputeAofs: the non-EB flux-register path ignores do_crse_add/do_fine_add. Honour both flags exactly as the EB branch does. do_fine_add is now used in every build config, so its ignore_unused is dropped.

#230 — an old-style ns.lo_bc/hi_bc entry silently overrides a conflicting xlo.type. Let the integer table fill in only when no string type was supplied (bc_type == "null"), so a mismatch reaches the existing "Multiple conflicting BCs" check. Also closes the related gap noted in the issue: lo_bc and hi_bc are now read as a pair, where previously ns.hi_bc on its own was parsed into nothing and silently dropped.

Also included

init_ConvectedVortex: meanFlowDir = +/-3 put the mean flow on x and y, not z (Source/prob/prob_init.cpp). 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. Now sets the z component only, matching the single-component pattern of case +/-1 and +/-2. Since AMREX_D_TERM drops the z term in 2D, the validation guard above is extended to abort on +/-3 in 2D rather than silently running as "no mean flow".

This one is in a separate commit (3fd14d64) since it did not come from a filed issue -- happy to split it out if you would rather keep this PR strictly issue-scoped.

Not included: #229

I implemented the suggested fix for #229 and it made things worse. amrex::volumeWeightedSum is EB-aware -- it has weighted by the volume fraction since AMReX 8b367b0071 (2022-09-25) -- so the patch double-counts vfrac. Covered cells are already handled too, since vfrac == 0 exactly. Details and measurements in #229 (comment); I suggest closing #229 as invalid.

Verification

Built Exec/run2d, Exec/eb_run2d (MPI, USE_EB=TRUE) and Exec/run3d (MPI) -- all clean.

• No answer change. regtest.2d.poiseuille (amr.max_level=1, so it exercises the ComputeAofs: non-EB flux-register path ignores do_crse_add/do_fine_add, so the mac_sync adds coarse fluxes the EB twin skips #226 mac_sync path) produces a plt00010 that is bit-identical to a from-scratch build of development. This confirms the issue's reasoning that the spurious CrseAdd was discarded before it was refluxed.

• Regression decks. 4/4 in Exec/run2d and 5/6 in Exec/eb_run2d run clean. eb_run2d/regtest.2d.shock_past_cylinder aborts with MLMG failed, but that is pre-existing -- it fails identically on unmodified development.

• main(): a run that sets only stop_interval aborts before stop_time is derived from it #222 -- stop_interval = 0.002 with max_step = -1 and no stop_time now runs and stops exactly at TIME = 0.002.

• Initialize_bcs: an old-style ns.lo_bc/hi_bc entry silently overrides a conflicting xlo.type, defeating the "Multiple conflicting BCs" check #230 -- ns.lo_bc = 5 combined with xlo.type = mass_inflow now aborts with Multiple conflicting BCs specified for xlo: 1, 5. Integer-only decks are unaffected (shock_past_cylinder still prints xlo set to mass inflow). ns.hi_bc without ns.lo_bc now aborts.

• meanFlowDir -- 3D sweep with prob.meanFlowMag = 50, as max|u| / max|v| / max|w|:

  meanFlowDir	before	after
  0	51.6 / 51.6 / 0	51.6 / 51.6 / 0
  1	101.6 / 51.6 / 0	101.6 / 51.6 / 0
  2	51.6 / 101.6 / 0	51.6 / 101.6 / 0
  3	101.6 / 101.6 / 0	51.6 / 51.6 / 50

  dir=0 shows the vortex alone contributes 51.6 to both u and v, so dir=1/2 add 50 to exactly one component and dir=3 now does the same on w.

#224 was not exercised at runtime -- I tried to induce a NaN with ns.fixed_dt = 50, but the run dies in MLMG before reaching the velocity-update check. The change is a one-line substitution that compiles, and amrex::Abort returning a nonzero status is demonstrated by the #230 tests above, but I did not prove that specific call site fires.

🤖 Generated with Claude Code

[cgilet]

The intent of meanFlowDir = 3 was as originally written, that is to create a mean flow that is not axis aligned. This problem is inherently 2D but can be run as pseudo 3D. I can see that meanFlowDir = 3 is confusing and suggest we could use 4 instead of 3.

[asalmgren]

Thanks — you are right, and I had this backwards. I read the old case 3 as a typo because it was the only branch that was not single-component, and I did not consider that non-axis-aligned was the point. Sorry for removing it.

Adopted your numbering in ff09d46: +/-4 is now the diagonal, restored as the pre-PR case +/-3 body verbatim, and +/-3 is the z-aligned flow.

One extension to your suggestion, which I am happy to drop if you disagree: I kept this 2D guard, but narrowed it to +/-3 only. Now that +/-3 means z, AMREX_D_TERM still drops it in 2D and the mean flow would vanish silently. The message now points at +/-4:

init_ConvectedVortex: prob.meanFlowDir = +/-3 puts the mean flow along z
and requires 3D.  Use +/-4 for a diagonal mean flow in the x-y plane.

+/-4 itself works fine in 2D, so the pseudo-2D use you describe is unaffected.

[cgilet]

Suggested change

	if ( IC.meanFlowDir < -3 || IC.meanFlowDir > 3 ) {
	if ( IC.meanFlowDir < -4 || IC.meanFlowDir > 4 ) {

[asalmgren]

Applied in ff09d46.

[cgilet]

Suggested change

	break;
	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 -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;

[asalmgren]

Applied in ff09d46, with case 4 / case -4 added after case -3 and before the case 0 / default fallthrough.

I applied it by hand rather than with the "commit suggestion" button: the suggested block has trailing whitespace after vel(i,j,k,2) = w_vort); on the case -4 line, and the repo's trailing_whitespaces style job would have failed on it.

Otherwise the body is your text unchanged, which also means it is character-identical to the pre-PR case +/-3.

[cgilet]

Suggested change

	amrex::Abort("\n init_ConvectedVortex: prob.meanFlowDir must be 0 (no mean flow) or +/-1, +/-2, +/-3\n in the inputs file.");
	amrex::Abort("\n init_ConvectedVortex: prob.meanFlowDir must be 0 (no mean flow) or +/-1, +/-2, +/-3, +/-4\n in the inputs file.");

[asalmgren]

Applied in ff09d46, reworded slightly to say what each value does rather than just listing them:

init_ConvectedVortex: prob.meanFlowDir must be 0 (no mean flow),
+/-1, +/-2, +/-3 (mean flow along x, y, z) or +/-4 (diagonal in the x-y plane)
in the inputs file.

I also filled in Docs/sphinx_documentation/source/ProblemSetup.rst, which described meanFlowDir only as "Direction of the mean flow the vortex is convected by" and listed no valid values at all.

[asalmgren]

@cgilet thanks for the review — pushed as ff09d46, replies on each thread.

Summary: you were right that the original meanFlowDir = 3 was intentional, not a bug. The encoding is now

value	mean flow
0	none (the vortex alone)
+/-1, +/-2, +/-3	along x, y, z (+/-3 is 3D only)
+/-4	diagonal in the x-y plane — the original behaviour, restored

+/-4 reproduces the old +/-3 exactly. Rather than take that on faith I rebuilt prob_init.cpp from development, ran the 3D ConvectedVortex tutorial with meanFlowDir = 3, and diffed against the new build with meanFlowDir = 4: plt00000 and plt00001 are both bit-identical. So an existing deck gets its old results back by changing 3 to 4.

Full 3D sweep at prob.meanFlowMag = 50, as max|u| / max|v| / max|w|:

  dir= 0 ->  51.6 /  51.6 /   0      dir= 3 ->  51.6 /  51.6 /  50
  dir= 1 -> 101.6 /  51.6 /   0      dir= 4 -> 101.6 / 101.6 /   0
  dir= 2 ->  51.6 / 101.6 /   0

2D: 0/1/2 unchanged, 4 and -4 give the diagonal, 3 aborts pointing at 4, 5 aborts on range. The 2D non-EB and EB regression decks all still run, and regtest.2d.poiseuille plt00010 is bit-identical to development. Both style scripts pass locally.

Two things I did not want to decide on my own:

1. The diagonal has |U| = meanFlowMag*sqrt(2), not meanFlowMag, since the magnitude is added to both components. I kept it exactly as it was — that is what makes 4 bit-match the old 3 — but if a unit-magnitude diagonal was the intent, that is a deliberate answer change and your call.
2. 3D decks that set meanFlowDir = 3 silently change meaning (diagonal -> z-aligned). Nothing in the repo is affected; both ConvectedVortex tutorials use 1. But an external deck would shift without warning. Happy to add a RELEASE_NOTES line if you think it is worth flagging.

Unrelated to your comments, for context on the rest of the PR: #229 is not included — volumeWeightedSum turned out to be EB-aware already, so the suggested fix there double-counted the volume fraction and made the answer worse. Details in #229 (comment).