IB: abort if an immersed boundary marks no cell anywhere - #1914
Open
sbryngelson wants to merge 1 commit into
Open
sbryngelson wants to merge 1 commit into
sbryngelson wants to merge 1 commit into
Conversation
A rank is given an IB patch when the patch centroid falls in its share of
the domain. For an STL the centroid and the geometry are independent: the
body is placed by model_translate, and a case may legitimately leave the
centroid at the origin. Ownership is then decided at a point the body does
not occupy, and the owning rank marks only whatever of the geometry its own
subdomain happens to reach.
That reach shrinks as the decomposition is refined, so a body erodes and can
vanish outright -- no markers, no ghost points, and a force of exactly zero
written every step, with nothing reported. Marker generation is a pure
function of geometry and grid, so a result that depends on the rank count is
always wrong.
Measured on a two-body case, identical deck, only the rank count changed,
with a patch at the origin as a control:
ranks control (at origin) patch 8 chords away
64 30191 cells, span 2.50 19115 cells, span 2.51
128 30191 cells, span 2.50 12667 cells, span 0.84
512 30191 cells, span 2.50 absent, force identically 0.0
This does not change the ownership rule -- using the centroid as the
placement point is a legitimate choice. It adds the invariant that choice
implies: every patch marks at least one cell somewhere. One reduction per
patch at setup, nothing at run time.
It catches total loss, not partial erosion; that needs the marked volume,
which is not available at this point.
Lines of Code
|
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The critical setup-time scaling and collective overhead must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Adds setup-time validation to abort when an immersed-boundary patch marks no cells globally.
Changes:
- Validates marker presence after IB generation.
- Counts markers per global patch using MPI reduction.
- Reports centroid/geometry mismatch guidance.
| File | Summary | Review |
|---|---|---|
src/simulation/m_ibm.fpp |
Adds global IB marker validation during setup. | Critical: scanning all local cells once per global patch and repeating collectives creates excessive setup cost; use a single count pass and reduction, or restrict scans to local IDs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1703
to
+1707
| do gid = 1, num_gbl_ibs | ||
| cnt_loc = 0_8 | ||
| do k = 0, p | ||
| do j = 0, n | ||
| do i = 0, m |
This branch has not been deployed
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.
The failure
A rank is given an IB patch when the patch centroid falls in its share of the domain. For an STL the centroid and the geometry are independent: the body is placed by
model_translate, and a case may legitimately leavepatch_ib%*_centroidat the origin. Ownership is then decided at a point the body does not occupy, and the owning rank marks only whatever of the geometry its own subdomain happens to reach.That reach shrinks as the decomposition is refined. A body erodes, and can vanish outright — no markers, no ghost points, and a force of exactly
0.0written to the force file every step, with nothing reported.Marker generation is a pure function of geometry and grid, so a result that depends on the rank count is always wrong.
Reproducer
Two-body case, identical input deck, only the rank count changed, with a patch at the origin as a built-in control:
The control is bit-identical at every decomposition; only the off-origin patch degrades.
What this changes
It does not change the ownership rule — using the centroid as the placement point is a legitimate design choice. It adds the invariant that choice implies: every global patch marks at least one cell somewhere. One integer reduction per patch at setup, nothing at run time, and a specific error message pointing at the centroid/geometry mismatch.
Limitation, stated plainly
This catches total loss, not the partial erosion visible at 128 ranks above. Detecting partial erosion needs the marked volume, which is not available at that point in setup. A user-side check on
lustre_ib.datcan compare marked volumes across rank counts if that matters; volume is the right invariant there, not cell count, since a patch in a coarser region of a stretched grid is correctly marked by fewer cells (in our case 37% fewer cells for 1.1% more volume).Verification
pre_process,simulation,post_process) build onmaster+ this patch../mfc.sh formatclean; all seven precheck gates pass.🤖 Generated with Claude Code