Skip to content

IncfloVelFill LOW K block inverts direction_dependent sign test (zlo inflow/outflow swapped) #190

Description

@WeiqunZhang

Severity: high · Category: correctness · Fix order: 6 of 29 — fix this 6th.

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

Location: src/prob/prob_bc.H:243

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

The defect

src/prob/prob_bc.H:243 — IncfloVelFill LOW K block inverts the direction_dependent sign test: it applies the Dirichlet inflow value when norm_vel <= 0 and copies interior (outflow treatment) when norm_vel > 0, the opposite of the LOW I (line 86) and LOW J (line 165) blocks; copy-pasted from the HIGH-face blocks.

src/prob/prob_bc.H:243 — IncfloVelFill's LOW K block inverts the direction_dependent test: line 243 applies Dirichlet when norm_vel <= 0 and line 251 extrapolates when norm_vel > 0 -- the HIGH-side conditions -- whereas LOW I (lines 85-86,94), LOW J (164-165,173), and the low-z branches of IncfloDenFill/TracFill/TempFill (lines 377,489,599) all use >= 0 / < 0.

Why it matters

F009: 3D run with zlo.type="direction_dependent" and zlo.velocity=(0,0,w>0) (inflow through bottom): velocity ghost cells at zlo are filled by copying interior values while IncfloDenFill/TracFill/TempFill (lines 377/489/599, all '>= 0.') fill inflow Dirichlet values -- inflow velocity BC silently never enforced; with w<0 (outflow) the Dirichlet value is wrongly forced.

F016: 3D run with zlo boundary type 'direction_dependent' ("dd"): inflow at zlo (w>0) is treated as outflow (ghost cells copied from interior) and outflow (w<0) is clamped to the Dirichlet inflow values -- boundary behavior is exactly reversed at the low-z face, unlike every other face.

How to reach it

  • 3D build, e.g. test_no_eb_3d/benchmark.inout modified: zlo.type="direction_dependent", zlo.velocity=0. 0. 1., zhi.type="po", x/y walls. No assert/abort gates it; shipped deck already uses direction_dependent on x faces.
  • 3D build; e.g. test_no_eb_3d/benchmark.inout with zlo.type="dd", zlo.velocity=0. 0. 1., zhi.type="po", geometry.is_periodic=1 1 0. No abort blocks it; LOW K executes on every velocity fillpatch.

Suggested fix

In the LOW K block of IncfloVelFill, flip the two sign tests to the low-side convention: gate the Dirichlet branch (line 243) on norm_vel >= 0. and the interior-copy branch (line 251) on norm_vel < 0.. On a low domain face the outward normal points in -z, so a positive normal velocity means flow entering the domain and must receive the Dirichlet inflow value; only negative normal velocity (outflow) should extrapolate from the interior. This is exactly the convention already used by the LOW I and LOW J twins in the same function, by the low-z branches of IncfloDenFill/IncfloTracFill/IncfloTempFill (lines 377/489/599), and by the MAC BC code (cc_arr(...,2) >= 0 at low z), and it matches the pre-#125 low-k code before commit 5634287 flipped it. The change restores intended behavior for all direction_dependent users at zlo; no compatibility decision is needed, though the maintainer may want a 3D regression test with zlo.type = "direction_dependent" since no existing test caught the flip.

For src/prob/prob_bc.H:243 (F009):

--- a/src/prob/prob_bc.H
+++ b/src/prob/prob_bc.H
@@ -240,15 +240,15 @@
                 }
 
                 if ( (bc.lo(dir) == amrex::BCType::ext_dir) ||
-                     (bc.lo(dir) == amrex::BCType::direction_dependent && norm_vel <= 0.) )
+                     (bc.lo(dir) == amrex::BCType::direction_dependent && norm_vel >= 0.) )
                 {
                     if ( orig_comp+nc == dir ) {
                         vel(i,j,k,dcomp+nc) = norm_vel;
                     } else {
                         vel(i,j,k,dcomp+nc) =  bcv_vel[amrex::Orientation(amrex::Direction::z,amrex::Orientation::low)][orig_comp+nc];
                     }
                 }
-                else if (bc.lo(dir) == amrex::BCType::direction_dependent && norm_vel > 0.)
+                else if (bc.lo(dir) == amrex::BCType::direction_dependent && norm_vel < 0.)
                 {
                     vel(i,j,k,dcomp+nc) = vel(i,j,domain_box.smallEnd(2),dcomp+nc);
                 }

Flips LOW K direction_dependent tests to match LOW I/LOW J and the Den/Trac/Temp low-z fills: Dirichlet when norm_vel >= 0., copy interior when norm_vel < 0. Overlaps F037's LOW-K hunk; apply one first and hand-merge the other (merged line 242-243: 'else if ( (...ext_dir) || (...direction_dependent && norm_vel >= 0.) )').

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

F009 — confirmed (two independent verifier lenses)

Lens 1 (refutation attempt): prob_bc.H:242-243: LOW K Dirichlet branch is '(bc.lo(dir) == amrex::BCType::direction_dependent && norm_vel <= 0.)' and line 251 copies interior when 'norm_vel > 0.' — reversed vs LOW I (line 86 '>= 0.'), LOW J (165), and IncfloDenFill/TracFill/TempFill low-z (377/489/599, all '>= 0.'). MAC code also uses 'cc_arr(...,2) >= 0' at low-z. Regressed in commit 5634287 (#125), unfixed at HEAD.

Lens 2 (reachability/intent): prob_bc.H:242-251 LOW K uses norm_vel<=0 for Dirichlet, >0 for interior copy — opposite of LOW I/J (86,165) and DenFill/TracFill/TempFill LOW K (377,489,599, all >=0). History: 60881d3 had LOW K correct (>=0); 5634287 pasted HIGH-face conditions (bc.hi, <=0) into LOW K; 8643ecc "fix typo" restored bc.lo but missed the signs. Consumed via IncfloVelFill in fillpatch, advection, and nodal-projection inflow enforcement (incflo_apply_nodal_projection.cpp:153).

F016 — confirmed (two independent verifier lenses)

Lens 1 (refutation attempt): Same defect as F009, confirmed in current source: line 243 'norm_vel <= 0.' gates Dirichlet and line 251 'norm_vel > 0.' gates interior copy in the LOW K block — the HIGH-side conditions. All other low-side twins (lines 86/94, 165/173) and scalar low-z fills (377/489/599) use '>= 0.'/'< 0.'. git show 5634287 shows the pre-#125 low-k code correctly used 'norm_vel >= 0.' for Dirichlet; #125 flipped it.

Lens 2 (reachability/intent): prob_bc.H:242-251 LOW K uses norm_vel <= 0.→Dirichlet / > 0.→copy-interior, mirror of LOW I/J (85-86,94) and scalar low-z fills (377,489,599). History: 60881d3 had LOW K correct (>= 0.); 5634287 (#125) pasted HIGH-side logic (bc.hi, <= 0.); 8643ecc (#126 "fix typo") fixed only bc.hibc.lo, leaving signs inverted. No assert gates dd; IncfloVelFill fills all velocity fillpatches (incflo_fillpatch.cpp:13-26).


Based on commit 7307d872, which is also the tree the audit verified against. From an automated audit of src/. Audit finding ids: F009, F016. Reviewer unit(s): Prob, theme:symmetry-twins. 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