Refuse a boundary-condition patch geometry the dimensionality never applies - #1869
Open
sbryngelson wants to merge 3 commits into
Open
sbryngelson wants to merge 3 commits into
sbryngelson wants to merge 3 commits into
Conversation
…pplies s_apply_boundary_patches dispatches by dimensionality -- geometry 1 in 2D, geometry 2 or 3 in 3D -- and has no else. A geometry belonging to the other case falls straight through: the patch is never applied, nothing is printed, and the face silently keeps whatever bc_[xyz] gave it. A nozzle cut into a no-slip wall with a 3D geometry in a 2D case therefore stays a solid wall. The run completes, writes output, and the jet has a velocity of exactly zero for all time with no indication anything was ignored. That is how it was found. examples/2D_bc_patch_geometry carries the case with a GEOMETRY knob: geometry 1 passes as before, GEOMETRY=3 now reports patch_bc(1)%geometry must be 1 (line segment) in 2D; geometry 3 is never applied where it used to pass and then do nothing. The 3D direction is symmetric and is refused the same way. Validator only; all 182 example cases still validate.
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds validator checks to prevent boundary-condition patch geometries that are ignored due to dimensionality-based dispatch, and introduces a minimal example case documenting the failure mode.
Changes:
- Add dimensionality-aware
patch_bc(i)%geometryvalidation in the case validator (2D: geometry 1; 3D: geometry 2/3). - Add a new 2D example (
2D_bc_patch_geometry) demonstrating the validator rejection for an invalid geometry. - Document the runtime “silent no-op” trap (and a related IC/Dirichlet-buffer trap) in the example README.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| toolchain/mfc/case_validator.py | Enforces geometry choices that the runtime dispatch actually applies for 2D vs 3D. |
| examples/2D_bc_patch_geometry/case.py | Adds a reproducible case with a GEOMETRY toggle to validate the new rule. |
| examples/2D_bc_patch_geometry/README.md | Documents the trap, how to reproduce it, and scope/limitations of the fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+2224
to
+2225
| p = self.get("p", 0) or 0 | ||
| n = self.get("n", 0) or 0 |
| if p > 0: | ||
| self.prohibit(geometry not in (2, 3), f"patch_bc({i})%geometry must be 2 (circle) or 3 (rectangle) in 3D; " f"geometry {geometry} is never applied") | ||
| elif n > 0: | ||
| self.prohibit(geometry != 1, f"patch_bc({i})%geometry must be 1 (line segment) in 2D; " f"geometry {geometry} is never applied") |
Comment on lines
+27
to
+30
| | | before | after | | ||
| | --- | --- | --- | | ||
| | `GEOMETRY=1` | passes | passes | | ||
| | `GEOMETRY=3` | **passes, then does nothing at run time** | `patch_bc(1)%geometry must be 1 (line segment) in 2D; geometry 3 is never applied` | |
Comment on lines
+2227
to
+2229
| self.prohibit(geometry not in (2, 3), f"patch_bc({i})%geometry must be 2 (circle) or 3 (rectangle) in 3D; " f"geometry {geometry} is never applied") | ||
| elif n > 0: | ||
| self.prohibit(geometry != 1, f"patch_bc({i})%geometry must be 1 (line segment) in 2D; " f"geometry {geometry} is never applied") |
The example was added without them, so every platform failed the same check -- "The golden file does not exist!" -- on all 26 jobs that run the suite. Generated on CPU and re-run without --generate to confirm it compares clean.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1869 +/- ##
==========================================
- Coverage 61.26% 61.25% -0.02%
==========================================
Files 84 84
Lines 22330 22336 +6
Branches 3265 3266 +1
==========================================
+ Hits 13680 13681 +1
- Misses 6207 6211 +4
- Partials 2443 2444 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Written with Claude Code.
The trap
s_apply_boundary_patches(src/pre_process/m_boundary_conditions.fpp) dispatches by dimensionality:There is no
else. A geometry belonging to the other dimensionality falls straight through — the patch is never applied, nothing is printed, and the face silently keeps whateverbc_[xyz]gave it.How it surfaced
A nozzle cut into a no-slip wall (
bc_x%beg = -16plus a Dirichletpatch_bc) in a 2D case, written with geometry 3. The run completed, wrote output, and the exit velocity was exactly 0.0000 for all time — the wall had simply stayed a wall. No warning, no error, nothing in the log. It took two runs and a read of the dispatch to find.The fix
A validator check that the geometry is one the dimensionality actually dispatches. Both directions, since the 3D case is symmetric.
How the validation is obtained
examples/2D_bc_patch_geometry/— the case that hit it, with aGEOMETRYknob:GEOMETRY=1GEOMETRY=3patch_bc(1)%geometry must be 1 (line segment) in 2D; geometry 3 is never appliedAll 182 example cases still validate — no existing case relies on a geometry that was being ignored, which I checked rather than assumed. Precheck is clean.
A second trap in the same corner, documented not fixed
The example carries a second initial-condition patch a few cells thick at the inlet, and it is load-bearing: the Dirichlet buffer is filled by pre_process from the initial condition at the boundary face, so a domain initialised at rest stores rest and the inflow delivers zero — again silently. The README says so, because from the outside the two failures look identical and knowing that saves working out which is in play. Not addressed here; it is a separate question about whether that should warn.
Scope
Validator and one example. No source change, no golden files.