Periodic Immersed Boundary Wrapping for Circular and Spherical Geometries#1117
Periodic Immersed Boundary Wrapping for Circular and Spherical Geometries#1117conraddelgado wants to merge 13 commits intoMFlowCode:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds support for periodic immersed boundaries: new Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1117 +/- ##
==========================================
+ Coverage 44.03% 44.15% +0.12%
==========================================
Files 70 70
Lines 20649 20757 +108
Branches 2053 2074 +21
==========================================
+ Hits 9093 9166 +73
- Misses 10368 10388 +20
- Partials 1188 1203 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| ! wrap ib centroid around domain after final stage in timestep | ||
| if (periodic_ibs .and. s == nstage) then | ||
| patch_ib(i)%x_centroid = domain_glb(1, 1) + modulo(patch_ib(i)%x_centroid - domain_glb(1, 1), domain_glb(1, 2) - domain_glb(1, 1)) | ||
| patch_ib(i)%y_centroid = domain_glb(2, 1) + modulo(patch_ib(i)%y_centroid - domain_glb(2, 1), domain_glb(2, 2) - domain_glb(2, 1)) | ||
| if (p > 0) then ! 3D | ||
| patch_ib(i)%z_centroid = domain_glb(3, 1) + modulo(patch_ib(i)%z_centroid - domain_glb(3, 1), domain_glb(3, 2) - domain_glb(3, 1)) | ||
| end if | ||
| end if |
There was a problem hiding this comment.
Suggestion: Modify the immersed boundary wrapping logic to execute at every Runge-Kutta sub-stage, not just the final one, by removing the s == nstage condition. [possible issue, importance: 7]
| ! wrap ib centroid around domain after final stage in timestep | |
| if (periodic_ibs .and. s == nstage) then | |
| patch_ib(i)%x_centroid = domain_glb(1, 1) + modulo(patch_ib(i)%x_centroid - domain_glb(1, 1), domain_glb(1, 2) - domain_glb(1, 1)) | |
| patch_ib(i)%y_centroid = domain_glb(2, 1) + modulo(patch_ib(i)%y_centroid - domain_glb(2, 1), domain_glb(2, 2) - domain_glb(2, 1)) | |
| if (p > 0) then ! 3D | |
| patch_ib(i)%z_centroid = domain_glb(3, 1) + modulo(patch_ib(i)%z_centroid - domain_glb(3, 1), domain_glb(3, 2) - domain_glb(3, 1)) | |
| end if | |
| end if | |
| ! wrap ib centroid around domain | |
| if (periodic_ibs) then | |
| patch_ib(i)%x_centroid = domain_glb(1, 1) + modulo(patch_ib(i)%x_centroid - domain_glb(1, 1), domain_glb(1, 2) - domain_glb(1, 1)) | |
| patch_ib(i)%y_centroid = domain_glb(2, 1) + modulo(patch_ib(i)%y_centroid - domain_glb(2, 1), domain_glb(2, 2) - domain_glb(2, 1)) | |
| if (p > 0) then ! 3D | |
| patch_ib(i)%z_centroid = domain_glb(3, 1) + modulo(patch_ib(i)%z_centroid - domain_glb(3, 1), domain_glb(3, 2) - domain_glb(3, 1)) | |
| end if | |
| end if |
There was a problem hiding this comment.
We can change this to wrap the centroids around the domain at every RK substep instead of only at the end of a timestep. It will just require also updating step_[x,y,z]_centroid as well.
There was a problem hiding this comment.
Code Implementation 🛠️
Implementation: Apply periodic immersed-boundary centroid wrapping at every Runge–Kutta sub-stage (not only the final stage) and keep the RK reference centroid values (step_x/y/z_centroid) consistent by wrapping them as well.
| ! wrap ib centroid around domain after final stage in timestep | |
| if (periodic_ibs .and. s == nstage) then | |
| patch_ib(i)%x_centroid = domain_glb(1, 1) + modulo(patch_ib(i)%x_centroid - domain_glb(1, 1), domain_glb(1, 2) - domain_glb(1, 1)) | |
| patch_ib(i)%y_centroid = domain_glb(2, 1) + modulo(patch_ib(i)%y_centroid - domain_glb(2, 1), domain_glb(2, 2) - domain_glb(2, 1)) | |
| if (p > 0) then ! 3D | |
| patch_ib(i)%z_centroid = domain_glb(3, 1) + modulo(patch_ib(i)%z_centroid - domain_glb(3, 1), domain_glb(3, 2) - domain_glb(3, 1)) | |
| end if | |
| end if | |
| ! wrap ib centroid around domain at every RK sub-stage | |
| if (periodic_ibs) then | |
| patch_ib(i)%x_centroid = domain_glb(1, 1) + modulo(patch_ib(i)%x_centroid - domain_glb(1, 1), domain_glb(1, 2) - domain_glb(1, 1)) | |
| patch_ib(i)%y_centroid = domain_glb(2, 1) + modulo(patch_ib(i)%y_centroid - domain_glb(2, 1), domain_glb(2, 2) - domain_glb(2, 1)) | |
| ! Keep RK reference centroid consistent with wrapped coordinates | |
| patch_ib(i)%step_x_centroid = domain_glb(1, 1) + modulo(patch_ib(i)%step_x_centroid - domain_glb(1, 1), domain_glb(1, 2) - domain_glb(1, 1)) | |
| patch_ib(i)%step_y_centroid = domain_glb(2, 1) + modulo(patch_ib(i)%step_y_centroid - domain_glb(2, 1), domain_glb(2, 2) - domain_glb(2, 1)) | |
| if (p > 0) then ! 3D | |
| patch_ib(i)%z_centroid = domain_glb(3, 1) + modulo(patch_ib(i)%z_centroid - domain_glb(3, 1), domain_glb(3, 2) - domain_glb(3, 1)) | |
| patch_ib(i)%step_z_centroid = domain_glb(3, 1) + modulo(patch_ib(i)%step_z_centroid - domain_glb(3, 1), domain_glb(3, 2) - domain_glb(3, 1)) | |
| end if | |
| end if |
See review comment here
There was a problem hiding this comment.
As its written, this will not change the value of step_[x,y,z]_centroid as long as they are within the computational domain. To update [x,y,z]centroid at every step we need to wrap step[x,y,z]_centroid such that it ends up outside the computational domain on the other side if [x,y,z]_centroid has been wrapped, which is why I originally only wrapped the centroids at the end of every timestep. It really should not matter as the ib_markers and levelset routines should still return the same values regardless.
There was a problem hiding this comment.
Question:
As its written, this will not change the value of step_[x,y,z]_centroid as long as they are within the computational domain. To update [x,y,z]centroid at every step we need to wrap step[x,y,z]_centroid such that it ends up outside the computational domain on the other side if [x,y,z]_centroid has been wrapped, which is why I originally only wrapped the centroids at the end of every timestep. It really should not matter as the ib_markers and levelset routines should still return the same values regardless.
Answer:
Add a repo-level instruction to prevent speculative changes to numerical-integration/sub-stepping invariants unless the model can prove semantic equivalence, and to explicitly reason about coupled “state vs reference” variables (like centroid vs step_centroid) before proposing wrapping changes.
Recommended .pr_agent.toml snippet:
[pr_code_suggestions]
extra_instructions = """\
- Be conservative with numerical time-integration logic (e.g., Runge–Kutta sub-stages). Do NOT suggest moving logic between sub-stages vs end-of-timestep unless you can clearly justify equivalence and identify all state variables that must be updated consistently.
- For periodic-wrapping changes: if a variable has both a "current" value and an RK reference/anchor (e.g., `x_centroid` and `step_x_centroid`), explicitly explain whether the reference must remain unwrapped (physical displacement continuity) or be wrapped, and why.
- If the correct behavior depends on solver conventions (e.g., whether `step_*` is an unwrapped reference used for displacement), ask a clarifying question instead of proposing a patch.
- Prefer “discussion-only” suggestions (explain the risk/tradeoff) over concrete code diffs when the change affects algorithmic invariants.
"""If you want to further reduce algorithmic “refactor” suggestions and keep only clear problems/bugs:
[pr_code_suggestions]
focus_only_on_problems = trueRelevant Sources:
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 22 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/common/m_ib_patches.fpp">
<violation number="1" location="src/common/m_ib_patches.fpp:657">
P2: In the periodic_ibs branch of s_ib_sphere, cylindrical grids (grid_geometry == 3) are no longer converted to cartesian coordinates before distance checks. This will misplace periodic spheres in cylindrical/axisymmetric geometries; mirror the cart_y/cart_z conversion used in the non-periodic branch.</violation>
</file>
<file name="toolchain/mfc/case_validator.py">
<violation number="1" location="toolchain/mfc/case_validator.py:360">
P2: Add a validation that periodic_ibs requires ib to be enabled; otherwise invalid configurations (periodic_ibs='T' with ib='F') will pass validation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
toolchain/mfc/case_validator.py (1)
347-370:⚠️ Potential issue | 🟠 MajorDimension-specific geometry check is too permissive for periodic_ibs.
The current rule accepts geometry 2 or 8 for any dimension. That allows spheres in 2D and circles in 3D to pass validation, which contradicts the intended constraints and can slip invalid configs through. Also, consider rejecting periodic_ibs when ib is false to avoid silent no-op configurations.🔧 Proposed fix
def check_ibm(self): """Checks constraints on Immersed Boundaries parameters""" ib = self.get('ib', 'F') == 'T' n = self.get('n', 0) + p = self.get('p', 0) num_ibs = self.get('num_ibs', 0) periodic_ibs = self.get('periodic_ibs', 'F') == 'T' @@ self.prohibit(not ib and num_ibs > 0, "num_ibs is set, but ib is not enabled") if periodic_ibs: + self.prohibit(not ib, + "periodic_ibs requires ib = T") for direction in ['x', 'y', 'z']: for end in ['beg', 'end']: bc_val = self.get(f'bc_{direction}%{end}') if bc_val is not None: self.prohibit(bc_val != -1, "periodic_ibs requires periodicity in all directions (all BCs should be -1)") for i in range(1, num_ibs+1): ib_geometry = self.get(f'patch_ib({i})%geometry') - self.prohibit(ib_geometry not in [2, 8], - "periodic_ibs requires all immersed boundaries to be circles (2D) or spheres (3D)") + if p == 0: + self.prohibit(ib_geometry != 2, + "periodic_ibs requires circles (geometry = 2) in 2D") + else: + self.prohibit(ib_geometry != 8, + "periodic_ibs requires spheres (geometry = 8) in 3D")src/simulation/m_start_up.fpp (1)
149-191:⚠️ Potential issue | 🔴 CriticalAdd
GPU_UPDATE(device='[periodic_ibs]')after namelist input is read in m_start_up.fpp.
The variableperiodic_ibsis read from the user_inputs namelist (lines 189-191) and used in GPU kernels withinm_compute_levelset.fpp(copyin at lines 68 and 585), but it is never synchronized to the GPU device. Without this update, GPU levelset calculations will always seeperiodic_ibs = .false.regardless of user input, causing incorrect IB wrapping behavior.
🤖 Fix all issues with AI agents
In `@examples/2D_ibm_circle_periodic/case.py`:
- Around line 38-40: The inline comment above the grid size (near Nx and Ny
variables) incorrectly refers to a "sphere"; update that comment to say "circle"
since this is a 2D case (e.g., change the comment string "# to fully resolve
requires 40-60 cells across sphere diameter" to use "circle" instead). Ensure
the wording remains accurate and brief and that it sits immediately above or
next to the Nx/Ny declarations (symbols: Nx, Ny).
In `@examples/3D_ibm_sphere_periodic/case.py`:
- Around line 38-41: The comment about required resolution is inconsistent with
the grid settings: with L = 1 and Nx = 127 (Ny = Nz = Nx), the sphere diameter
covers roughly 13 cells, not 40–60; update the comment above the Nx/Ny/Nz
definitions (referencing Nx, Ny, Nz and L) to either state the actual
cells-per-diameter for this grid (~13) or note that 40–60 cells would require
increasing Nx accordingly, so the comment accurately reflects the chosen
resolution.
In `@src/common/m_ib_patches.fpp`:
- Around line 649-666: The periodic GPU path in the GPU_PARALLEL_LOOP uses x_cc,
y_cc, z_cc directly when computing distances and therefore skips
cylindrical-to-Cartesian conversion; update the periodic branch to perform the
same s_convert_cylindrical_to_cartesian_coord(center, center_cart) conversion
used in the non-periodic path (or compute center_cart before entering the loop)
and then use center_cart(1,ix)/center_cart(2,iy)/center_cart(3,iz) in the
distance test that assigns ib_markers_sf(i,j,k)=patch_id (preserving r2 and
patch_id usage), ensuring the cylindrical grid_geometry==3 case is handled
identically inside the periodic loop.
In `@src/common/m_mpi_common.fpp`:
- Around line 1895-1924: The subroutine s_mpi_global_domain_bounds allocates
domain_glb but never frees it; add a guarded deallocation in
s_finalize_mpi_common_module to avoid leaks/double-init cycles by checking that
domain_glb is allocated (e.g., using the ALLOCATED intrinsic) and then
CALL/DEALLOCATE (or DEALLOCATE) it safely, and ensure the finalize routine
references the same symbol name domain_glb and follows existing module
finalization ordering/guards so multiple finalize calls are safe.
In `@src/simulation/m_ibm.fpp`:
- Around line 156-231: The GPU kernels that populate periodic buffers (the
$:GPU_PARALLEL_LOOP blocks used around loops filling ib_markers%sf for x/y/z
directions when bc_x%beg/end, bc_y%beg/end, bc_z%beg/end are BC_PERIODIC) are
missing private lists for loop indices, so loop variables (e.g., j, k, l) can be
shared across threads; update each $:GPU_PARALLEL_LOOP invocation to include
private='[j,k,l]' (or the exact loop indices used in that block) so all
loop-local indices are declared private for the GPU kernels.
- Around line 207-231: The z-direction periodic buffer-fill writes
negative/positive z indices even when p == 0 (2D runs), causing out-of-bounds
access; guard the entire z-periodic block by checking p > 0 (or num_dims == 3)
before evaluating bc_z%beg/end so the loops that reference
ib_markers%sf(k,l,...) only run for 3D cases. Wrap the two if blocks that test
bc_z%beg==BC_PERIODIC and bc_z%end==BC_PERIODIC with a surrounding condition
(e.g., if (p > 0) then) so the code using ib_markers%sf and p, buff_size, k, l,
j is skipped for 2D runs.
In `@src/simulation/m_time_steppers.fpp`:
- Around line 825-831: The wrap for immersed boundary centroids uses
domain_glb(2,*) unconditionally, which will out-of-bounds in 1D; modify the
periodic_ib wrapping block (the if (periodic_ibs .and. s == nstage) branch that
updates patch_ib(i)%x_centroid, %y_centroid, and %z_centroid) to only perform
the y wrapping when num_dims > 1 (e.g., if (n > 0) then) and keep the existing
3D z guard (if (p > 0) then) so domain_glb indexing is safe for 1D, 2D and 3D
cases.
In `@tests/FF27BC5E/golden-metadata.txt`:
- Around line 1-193: The golden metadata includes a volatile "Git: ... (dirty)"
entry which causes flaky comparisons; update the metadata generator to normalize
the Git field (the "Git:" line) by either omitting the "(dirty)" marker or
writing only the commit hash (without workspace cleanliness), or regenerate the
golden from a clean working tree; change the code that emits the Git line so it
strips the "(dirty)" suffix or uses git rev-parse --short HEAD to always produce
a stable value.
🧹 Nitpick comments (1)
tests/60AD8A56/golden-metadata.txt (1)
5-33: Consider normalizing machine-specific metadata.Fields like absolute paths, hostnames, and “git dirty” can make golden files noisy across machines; consider redacting or normalizing these in the generator to reduce churn.
|
@danieljvickers can you help review plz |
|
@conraddelgado you should go through the ai suggestions and comment on them or fix them or dismiss them 😃 . i know it's a little annoying but it helps us catch bugs or just not great code |
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
bot
added
the
size:XXL
Nitpicks 🔍
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
CodeAnt AI finished reviewing your PR. |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
bot
added
size:XXL
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/simulation/m_global_parameters.fpp (2)
1421-1429:⚠️ Potential issue | 🟠 MajorFix
domain_glbdeallocation for 1D/2D runs (early return skips cleanup).
domain_glbis deallocated afterreturnguards; in 1D (n == 0) and 2D (p == 0), it never gets freed. Move the deallocation before the early returns or replace returns with conditional blocks.🛠️ Proposed fix
- @:DEALLOCATE(x_cb, x_cc, dx) - - if (n == 0) return; - @:DEALLOCATE(y_cb, y_cc, dy) - - if (p == 0) return; - @:DEALLOCATE(z_cb, z_cc, dz) - - @:DEALLOCATE(domain_glb) + @:DEALLOCATE(x_cb, x_cc, dx) + @:DEALLOCATE(domain_glb) + + if (n /= 0) then + @:DEALLOCATE(y_cb, y_cc, dy) + if (p /= 0) then + @:DEALLOCATE(z_cb, z_cc, dz) + end if + end if
399-412:⚠️ Potential issue | 🟠 MajorAdd
GPU_UPDATEcall to synchronize IB parameters after initialization.The IB parameters (
ib,num_ibs,patch_ib,periodic_ibs) are declared withGPU_DECLARE(create='[...]')at line 411, but they are initialized at lines 716–718 without a correspondingGPU_UPDATEcall. This leaves the device with uninitialized values. Add$:GPU_UPDATE(device='[ib,num_ibs,patch_ib,periodic_ibs]')immediately after line 718 to ensure the device sees the correct runtime values.
🧹 Nitpick comments (1)
src/pre_process/m_global_parameters.fpp (1)
306-308: Use fypp allocation macros fordomain_glbto align withsrc/**/*.fpprules.This file is under
src/, so prefer@:ALLOCATE/@:DEALLOCATEfor device-aware allocation.♻️ Proposed changes
@@ -#:include 'case.fpp' +#:include 'case.fpp' +#:include 'macros.fpp'- allocate (domain_glb(num_dims, 2)) + @:ALLOCATE(domain_glb(num_dims, 2)) @@ - deallocate (domain_glb) + @:DEALLOCATE(domain_glb)Also applies to: 979-989, 1046-1046
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
bot
added
size:XXL
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
toolchain/mfc/case_validator.py (1)
347-375:⚠️ Potential issue | 🟠 MajorEnforce dimension-specific periodic IB geometries (circle in 2D, sphere in 3D).
The current check allows geometry 2 or 8 regardless of dimensionality, so a 3D case could pass with a circle (or 2D with a sphere), contradicting the stated constraints and likely breaking downstream assumptions.✅ Suggested fix
- n = self.get('n', 0) + n = self.get('n', 0) + p = self.get('p', 0) num_ibs = self.get('num_ibs', 0) periodic_ibs = self.get('periodic_ibs', 'F') == 'T' cyl_coord = self.get('cyl_coord', 'F') == 'T' @@ - for i in range(1, num_ibs+1): + expected_geom = 8 if (p is not None and p > 0) else 2 + for i in range(1, num_ibs + 1): ib_geometry = self.get(f'patch_ib({i})%geometry') - self.prohibit(ib_geometry not in [2, 8], - "periodic_ibs requires all immersed boundaries to be circles (2D) or spheres (3D)") + self.prohibit(ib_geometry != expected_geom, + "periodic_ibs requires circles (2) in 2D and spheres (8) in 3D")src/simulation/m_global_parameters.fpp (1)
1438-1447:⚠️ Potential issue | 🟡 Minordomain_glb never deallocates in 1D/2D due to early returns.
The earlyreturnstatements exit beforedomain_glbis released, so 1D/2D runs leak the allocation.🛠️ Suggested fix
- @:DEALLOCATE(x_cb, x_cc, dx) - - if (n == 0) return; - @:DEALLOCATE(y_cb, y_cc, dy) - - if (p == 0) return; - @:DEALLOCATE(z_cb, z_cc, dz) - - @:DEALLOCATE(domain_glb) + @:DEALLOCATE(x_cb, x_cc, dx) + if (n > 0) then + @:DEALLOCATE(y_cb, y_cc, dy) + end if + if (p > 0) then + @:DEALLOCATE(z_cb, z_cc, dz) + end if + @:DEALLOCATE(domain_glb)
🤖 Fix all issues with AI agents
In `@src/simulation/m_global_parameters.fpp`:
- Around line 558-561: CI reports a formatting mismatch around the domain_glb
declaration; to fix it, re-run the project's code formatter on the file
containing the declaration of domain_glb (the allocatable real(wp), dimension(:,
:) :: domain_glb and its GPU_DECLARE directive) so the spacing/indentation and
continuation markers are normalized, then commit the formatted file to clear the
Pretty pipeline failure.
|
CodeAnt AI Incremental review completed. |
2 participants
User description
User description
Description
Adds periodic wrapping for circular and spherical immersed boundaries.
Fixes #(issue) [optional]
Type of change
Please delete options that are not relevant.
Scope
If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration
Tested 2D case (circle) and 3D case (sphere): in both cases the circle/sphere is placed in the corner of the domain and the flow field is compared against a simulation where the circle/sphere is located in the center of the domain. The flow fields should be exactly the same but offset.
2D circle located at center of domain - x velocity across diagonal
2D circle located at corner of domain - x velocity across diagonal
2D circle located at center of domain - x velocity video
2d_circle_verification_center.mp4
2D circle located at corner of domain - x velocity video
2d_circle_verification_corner.mp4
3D sphere located at center of domain - x velocity across diagonal
3D sphere located at corner of domain - x velocity across diagonal
3D sphere located at center of domain - x velocity video
3d_sphere_verification_center.mp4
3D sphere located at corner of domain - x velocity video
3d_sphere_verification_corner.mp4
Added the following examples
Test Configuration:
Ubuntu, MacOs
Checklist
docs/)examples/that demonstrate my new feature performing as expected.They run to completion and demonstrate "interesting physics"
./mfc.sh formatbefore committing my codeIf your code changes any code source files (anything in
src/simulation)To make sure the code is performing as expected on GPU devices, I have:
nvtxranges so that they can be identified in profiles./mfc.sh run XXXX --gpu -t simulation --nsys, and have attached the output file (.nsys-rep) and plain text results to this PR./mfc.sh run XXXX --gpu -t simulation --rsys --hip-trace, and have attached the output file and plain text results to this PR.PR Type
Enhancement
Description
Implements periodic wrapping for circular and spherical immersed boundaries
Adds global domain bounds tracking and periodic boundary synchronization
Extends levelset computation to handle periodically wrapped geometries
Includes 2D circle and 3D sphere example cases with validation tests
Diagram Walkthrough
File Walkthrough
8 files
Periodic wrapping logic for circle and sphere patchesLevelset computation with periodic distance calculationsBuffer population for periodic immersed boundariesIB centroid wrapping after final RK stageGlobal domain bounds computation and storageInitialize global domain bounds during startupInitialize global domain bounds in preprocessingValidation rules for periodic immersed boundaries8 files
Add periodic_ibs flag to global parametersBroadcast periodic_ibs parameter across MPI ranksBroadcast periodic_ibs in preprocessing stageAdd periodic_ibs flag to preprocessing parametersBroadcast periodic_ibs in postprocessing stageInclude periodic_ibs in postprocessing startupAdd periodic_ibs flag to postprocessing parametersAdd periodic_ibs parameter to case dictionary4 files
2D periodic circle immersed boundary test case3D periodic sphere immersed boundary test caseTest metadata for 2D periodic circle caseTest metadata for 3D periodic sphere case2 files
Summary by CodeRabbit
New Features
Tests
CodeAnt-AI Description
Periodic wrapping for circular and spherical immersed boundaries
What Changed
Impact
✅ Immersed boundaries wrap across periodic domain boundaries✅ Identical flow for an object at corner vs center when using periodic IBs✅ Clearer errors when periodic IBs are misconfigured💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.