Periodic Immersed Boundary Wrapping for Circular and Spherical Geometries #1117
+1,076
−70
Conversation
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
Contributor
📝 WalkthroughWalkthroughImplements periodic immersed boundary (IB) support by adding a Changes
Sequence DiagramsequenceDiagram
actor User
participant Startup as Start-up Module
participant MPI as MPI Module
participant GlobalParams as Global Parameters
participant Geometry as Geometry/Levelset
participant Patches as IB Patches
participant Buffers as IB Buffers
participant TimeStepper as Time Stepper
User->>Startup: Read input file (periodic_ibs flag)
Startup->>GlobalParams: Initialize periodic_ibs
Startup->>MPI: Call s_mpi_global_domain_bounds()
MPI->>GlobalParams: Populate domain_glb with global bounds
MPI->>Startup: Return domain_glb
Startup->>Geometry: Compute levelset with periodic wrapping
rect rgba(100, 150, 200, 0.5)
Geometry->>Geometry: If periodic_ibs: compute wrapped centers<br/>(original + periodic images)
Geometry->>Geometry: Find min distance across all images
end
Startup->>Patches: Apply IB patches with periodic wrapping
rect rgba(100, 150, 200, 0.5)
Patches->>Patches: If periodic_ibs: check 2D/3D grid of centers<br/>(original + wrapped positions)
Patches->>Patches: Mark intersecting cells across all images
end
Startup->>Buffers: Populate IB boundary buffers
rect rgba(100, 150, 200, 0.5)
Buffers->>Buffers: If periodic_ibs: populate periodic<br/>boundaries in x/y/z directions
end
Startup->>TimeStepper: Begin time integration
loop Each time stage
TimeStepper->>TimeStepper: Propagate IB centroids
rect rgba(150, 100, 150, 0.5)
alt If final stage and periodic_ibs
TimeStepper->>GlobalParams: Read domain_glb
TimeStepper->>TimeStepper: Wrap centroids back into domain<br/>using modulo arithmetic
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
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.16% 44.23% +0.07%
==========================================
Files 71 71
Lines 20417 20554 +137
Branches 1991 2018 +27
==========================================
+ Hits 9018 9093 +75
- Misses 10252 10299 +47
- Partials 1147 1162 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
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}
Contributor
There was a problem hiding this comment.
High-level Suggestion
The logic for calculating periodic "ghost" centers for immersed boundaries is duplicated in four subroutines. This should be extracted into a single helper function to reduce redundancy and improve maintainability. [High-level, importance: 7]
Solution Walkthrough:
Before:
subroutine s_ib_sphere(...)
...
if (periodic_ibs) then
! Logic to calculate ghost center for x-dimension
if ((center(1, 1) - domain_glb(1, 1)) <= radius) then
center(1, 2) = domain_glb(1, 2) + (center(1, 1) - domain_glb(1, 1))
else if ((domain_glb(1, 2) - center(1, 1)) <= radius) then
center(1, 2) = domain_glb(1, 1) - (domain_glb(1, 2) - center(1, 1))
else
center(1, 2) = center(1, 1)
end if
! ... similar duplicated logic for y-dimension ...
! ... similar duplicated logic for z-dimension ...
end if
...
end subroutine
! This entire if-block is repeated in 3 other subroutines.
After:
function get_periodic_centers(c, dim_idx, radius)
! Input: original center `c`, dimension index `dim_idx`, `radius`
! Output: array with original and ghost center
...
if ((c - domain_glb(dim_idx, 1)) <= radius) then
result(2) = domain_glb(dim_idx, 2) + (c - domain_glb(dim_idx, 1))
else if ((domain_glb(dim_idx, 2) - c) <= radius) then
result(2) = domain_glb(dim_idx, 1) - (domain_glb(dim_idx, 2) - c)
else
result(2) = c
end if
...
end function
subroutine s_ib_sphere(...)
...
if (periodic_ibs) then
center(1, :) = get_periodic_centers(patch_ib(patch_id)%x_centroid, 1, radius)
center(2, :) = get_periodic_centers(patch_ib(patch_id)%y_centroid, 2, radius)
center(3, :) = get_periodic_centers(patch_ib(patch_id)%z_centroid, 3, radius)
end if
...
end subroutine
Comment on lines
+649
to
+666
| $:GPU_PARALLEL_LOOP(collapse=3, private='[i,j,k,ix,iy,iz]', copy='[ib_markers_sf]', & | ||
| & copyin='[patch_id,center,r2]') | ||
| do k = 0, p | ||
| do j = 0, n | ||
| do i = 0, m | ||
| do ix = 1, 2 | ||
| do iy = 1, 2 | ||
| do iz = 1, 2 | ||
| if (((x_cc(i) - center(1, ix))**2 + (y_cc(j) - center(2, iy))**2 + (z_cc(k) - center(3, iz))**2) <= r2) then | ||
| ib_markers_sf(i, j, k) = patch_id | ||
| end if | ||
| end do | ||
| end do | ||
| end do | ||
| end do | ||
| end do | ||
| end do | ||
| end do | ||
| $:END_GPU_PARALLEL_LOOP() | ||
| $:END_GPU_PARALLEL_LOOP() |
Contributor
There was a problem hiding this comment.
Suggestion: Add support for cylindrical coordinates to the periodic sphere logic by including the s_convert_cylindrical_to_cartesian_coord call, ensuring correct behavior on cylindrical grids. [possible issue, importance: 9]
Suggested change
| $:GPU_PARALLEL_LOOP(collapse=3, private='[i,j,k,ix,iy,iz]', copy='[ib_markers_sf]', & | |
| & copyin='[patch_id,center,r2]') | |
| do k = 0, p | |
| do j = 0, n | |
| do i = 0, m | |
| do ix = 1, 2 | |
| do iy = 1, 2 | |
| do iz = 1, 2 | |
| if (((x_cc(i) - center(1, ix))**2 + (y_cc(j) - center(2, iy))**2 + (z_cc(k) - center(3, iz))**2) <= r2) then | |
| ib_markers_sf(i, j, k) = patch_id | |
| end if | |
| end do | |
| end do | |
| end do | |
| end do | |
| end do | |
| end do | |
| end do | |
| $:END_GPU_PARALLEL_LOOP() | |
| $:END_GPU_PARALLEL_LOOP() | |
| $:GPU_PARALLEL_LOOP(collapse=3, private='[i,j,k,ix,iy,iz,cart_y,cart_z]', copy='[ib_markers_sf]', & | |
| & copyin='[patch_id,center,r2,grid_geometry]') | |
| do k = 0, p | |
| do j = 0, n | |
| do i = 0, m | |
| if (grid_geometry == 3) then | |
| call s_convert_cylindrical_to_cartesian_coord(y_cc(j), z_cc(k)) | |
| else | |
| cart_y = y_cc(j) | |
| cart_z = z_cc(k) | |
| end if | |
| do ix = 1, 2 | |
| do iy = 1, 2 | |
| do iz = 1, 2 | |
| if (((x_cc(i) - center(1, ix))**2 + (cart_y - center(2, iy))**2 + (cart_z - center(3, iz))**2) <= r2) then | |
| ib_markers_sf(i, j, k) = patch_id | |
| end if | |
| end do | |
| end do | |
| end do | |
| end do | |
| end do | |
| end do | |
| $:END_GPU_PARALLEL_LOOP() |
Comment on lines
+825
to
+832
| ! 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 |
Contributor
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]
Suggested change
| ! 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 |
Comment on lines
+360
to
+366
| if periodic_ibs: | ||
| 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)") |
Contributor
There was a problem hiding this comment.
Suggestion: Refactor the periodic_ibs validation logic to check for periodic boundary conditions based on the simulation's dimensionality (m, n, p) instead of unconditionally looping through all three directions. [general, importance: 4]
Suggested change
| if periodic_ibs: | |
| 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)") | |
| if periodic_ibs: | |
| # Check X direction | |
| for end in ['beg', 'end']: | |
| self.prohibit(self.get(f'bc_x%{end}') != -1, | |
| "periodic_ibs requires periodicity in all directions (all BCs should be -1)") | |
| # Check Y direction (2D+) | |
| if self.get('n', 0) > 0: | |
| for end in ['beg', 'end']: | |
| self.prohibit(self.get(f'bc_y%{end}') != -1, | |
| "periodic_ibs requires periodicity in all directions (all BCs should be -1)") | |
| # Check Z direction (3D) | |
| if self.get('p', 0) > 0: | |
| for end in ['beg', 'end']: | |
| self.prohibit(self.get(f'bc_z%{end}') != -1, | |
| "periodic_ibs requires periodicity in all directions (all BCs should be -1)") |
|
|
||
| Invocation: test --generate -f 60AD8A56 -t 60AD8A56 | ||
| Lock: mpi=Yes & gpu=No & debug=No & gcov=No & unified=No & single=No & mixed=No & fastmath=No | ||
| Git: 1aa4b082059aacbb5d9a0183076414923e9f6044 on master (dirty) |
Contributor
There was a problem hiding this comment.
Suggestion: Enforce generating golden files from a clean git state to ensure test reproducibility by modifying the test generation script to abort if the repository is dirty. [general, importance: 7]
Suggested change
| Git: 1aa4b082059aacbb5d9a0183076414923e9f6044 on master (dirty) | |
| Git: 1aa4b082059aacbb5d9a0183076414923e9f6044 on master |
Comment on lines
+1
to
+193
| This file was created on 2026-01-29 18:26:42.881513. | ||
|
|
||
| mfc.sh: | ||
|
|
||
| Invocation: test --generate -f 60AD8A56 -t 60AD8A56 | ||
| Lock: mpi=Yes & gpu=No & debug=No & gcov=No & unified=No & single=No & mixed=No & fastmath=No | ||
| Git: 1aa4b082059aacbb5d9a0183076414923e9f6044 on master (dirty) | ||
|
|
||
| pre_process: | ||
|
|
||
| CMake Configuration: | ||
|
|
||
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | ||
|
|
||
| C : GNU v13.3.0 (/usr/bin/cc) | ||
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | ||
|
|
||
| PRE_PROCESS : ON | ||
| SIMULATION : OFF | ||
| POST_PROCESS : OFF | ||
| SYSCHECK : OFF | ||
| DOCUMENTATION : OFF | ||
| ALL : OFF | ||
|
|
||
| MPI : ON | ||
| OpenACC : OFF | ||
| OpenMP : OFF | ||
|
|
||
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | ||
| Doxygen : | ||
|
|
||
| Build Type : Release | ||
|
|
||
| Configuration Environment: | ||
|
|
||
| CC : /usr/bin/cc | ||
| CXX : /usr/bin/c++ | ||
| FC : /usr/bin/gfortran | ||
| OMPI_CC : | ||
| OMPI_CXX : | ||
| OMPI_FC : | ||
|
|
||
| simulation: | ||
|
|
||
| CMake Configuration: | ||
|
|
||
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | ||
|
|
||
| C : GNU v13.3.0 (/usr/bin/cc) | ||
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | ||
|
|
||
| PRE_PROCESS : OFF | ||
| SIMULATION : ON | ||
| POST_PROCESS : OFF | ||
| SYSCHECK : OFF | ||
| DOCUMENTATION : OFF | ||
| ALL : OFF | ||
|
|
||
| MPI : ON | ||
| OpenACC : OFF | ||
| OpenMP : OFF | ||
|
|
||
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | ||
| Doxygen : | ||
|
|
||
| Build Type : Release | ||
|
|
||
| Configuration Environment: | ||
|
|
||
| CC : /usr/bin/cc | ||
| CXX : /usr/bin/c++ | ||
| FC : /usr/bin/gfortran | ||
| OMPI_CC : | ||
| OMPI_CXX : | ||
| OMPI_FC : | ||
|
|
||
| post_process: | ||
|
|
||
| CMake Configuration: | ||
|
|
||
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | ||
|
|
||
| C : GNU v13.3.0 (/usr/bin/cc) | ||
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | ||
|
|
||
| PRE_PROCESS : OFF | ||
| SIMULATION : OFF | ||
| POST_PROCESS : ON | ||
| SYSCHECK : OFF | ||
| DOCUMENTATION : OFF | ||
| ALL : OFF | ||
|
|
||
| MPI : ON | ||
| OpenACC : OFF | ||
| OpenMP : OFF | ||
|
|
||
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | ||
| Doxygen : | ||
|
|
||
| Build Type : Release | ||
|
|
||
| Configuration Environment: | ||
|
|
||
| CC : /usr/bin/cc | ||
| CXX : /usr/bin/c++ | ||
| FC : /usr/bin/gfortran | ||
| OMPI_CC : | ||
| OMPI_CXX : | ||
| OMPI_FC : | ||
|
|
||
| syscheck: | ||
|
|
||
| CMake Configuration: | ||
|
|
||
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | ||
|
|
||
| C : GNU v13.3.0 (/usr/bin/cc) | ||
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | ||
|
|
||
| PRE_PROCESS : OFF | ||
| SIMULATION : OFF | ||
| POST_PROCESS : OFF | ||
| SYSCHECK : ON | ||
| DOCUMENTATION : OFF | ||
| ALL : OFF | ||
|
|
||
| MPI : ON | ||
| OpenACC : OFF | ||
| OpenMP : OFF | ||
|
|
||
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | ||
| Doxygen : | ||
|
|
||
| Build Type : Release | ||
|
|
||
| Configuration Environment: | ||
|
|
||
| CC : /usr/bin/cc | ||
| CXX : /usr/bin/c++ | ||
| FC : /usr/bin/gfortran | ||
| OMPI_CC : | ||
| OMPI_CXX : | ||
| OMPI_FC : | ||
|
|
||
| CPU: | ||
|
|
||
| CPU Info: | ||
| From lscpu | ||
| Architecture: x86_64 | ||
| CPU op-mode(s): 32-bit, 64-bit | ||
| Address sizes: 46 bits physical, 48 bits virtual | ||
| Byte Order: Little Endian | ||
| CPU(s): 32 | ||
| On-line CPU(s) list: 0-31 | ||
| Vendor ID: GenuineIntel | ||
| Model name: 13th Gen Intel(R) Core(TM) i9-13900K | ||
| CPU family: 6 | ||
| Model: 183 | ||
| Thread(s) per core: 2 | ||
| Core(s) per socket: 24 | ||
| Socket(s): 1 | ||
| Stepping: 1 | ||
| CPU(s) scaling MHz: 23% | ||
| CPU max MHz: 5800.0000 | ||
| CPU min MHz: 800.0000 | ||
| BogoMIPS: 5990.40 | ||
| Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf tsc_known_freq pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb ssbd ibrs ibpb stibp ibrs_enhanced tpr_shadow flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt clwb intel_pt sha_ni xsaveopt xsavec xgetbv1 xsaves split_lock_detect user_shstk avx_vnni dtherm ida arat pln pts hwp hwp_notify hwp_act_window hwp_epp hwp_pkg_req hfi vnmi umip pku ospke waitpkg gfni vaes vpclmulqdq rdpid movdiri movdir64b fsrm md_clear serialize pconfig arch_lbr ibt flush_l1d arch_capabilities | ||
| Virtualization: VT-x | ||
| L1d cache: 896 KiB (24 instances) | ||
| L1i cache: 1.3 MiB (24 instances) | ||
| L2 cache: 32 MiB (12 instances) | ||
| L3 cache: 36 MiB (1 instance) | ||
| NUMA node(s): 1 | ||
| NUMA node0 CPU(s): 0-31 | ||
| Vulnerability Gather data sampling: Not affected | ||
| Vulnerability Ghostwrite: Not affected | ||
| Vulnerability Indirect target selection: Not affected | ||
| Vulnerability Itlb multihit: Not affected | ||
| Vulnerability L1tf: Not affected | ||
| Vulnerability Mds: Not affected | ||
| Vulnerability Meltdown: Not affected | ||
| Vulnerability Mmio stale data: Not affected | ||
| Vulnerability Reg file data sampling: Mitigation; Clear Register File | ||
| Vulnerability Retbleed: Not affected | ||
| Vulnerability Spec rstack overflow: Not affected | ||
| Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl | ||
| Vulnerability Spectre v1: Mitigation; usercopy/swapgs barriers and __user pointer sanitization | ||
| Vulnerability Spectre v2: Mitigation; Enhanced / Automatic IBRS; IBPB conditional; PBRSB-eIBRS SW sequence; BHI BHI_DIS_S | ||
| Vulnerability Srbds: Not affected | ||
| Vulnerability Tsa: Not affected | ||
| Vulnerability Tsx async abort: Not affected | ||
| Vulnerability Vmscape: Mitigation; IBPB before exit to userspace | ||
|
|
Contributor
There was a problem hiding this comment.
Suggestion: Remove or normalize environment-specific information like timestamps, hostnames, user paths, and CPU details from the golden metadata file to prevent spurious test failures across different machines. [general, importance: 8]
Suggested change
| This file was created on 2026-01-29 18:26:42.881513. | |
| mfc.sh: | |
| Invocation: test --generate -f 60AD8A56 -t 60AD8A56 | |
| Lock: mpi=Yes & gpu=No & debug=No & gcov=No & unified=No & single=No & mixed=No & fastmath=No | |
| Git: 1aa4b082059aacbb5d9a0183076414923e9f6044 on master (dirty) | |
| pre_process: | |
| CMake Configuration: | |
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | |
| C : GNU v13.3.0 (/usr/bin/cc) | |
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | |
| PRE_PROCESS : ON | |
| SIMULATION : OFF | |
| POST_PROCESS : OFF | |
| SYSCHECK : OFF | |
| DOCUMENTATION : OFF | |
| ALL : OFF | |
| MPI : ON | |
| OpenACC : OFF | |
| OpenMP : OFF | |
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | |
| Doxygen : | |
| Build Type : Release | |
| Configuration Environment: | |
| CC : /usr/bin/cc | |
| CXX : /usr/bin/c++ | |
| FC : /usr/bin/gfortran | |
| OMPI_CC : | |
| OMPI_CXX : | |
| OMPI_FC : | |
| simulation: | |
| CMake Configuration: | |
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | |
| C : GNU v13.3.0 (/usr/bin/cc) | |
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | |
| PRE_PROCESS : OFF | |
| SIMULATION : ON | |
| POST_PROCESS : OFF | |
| SYSCHECK : OFF | |
| DOCUMENTATION : OFF | |
| ALL : OFF | |
| MPI : ON | |
| OpenACC : OFF | |
| OpenMP : OFF | |
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | |
| Doxygen : | |
| Build Type : Release | |
| Configuration Environment: | |
| CC : /usr/bin/cc | |
| CXX : /usr/bin/c++ | |
| FC : /usr/bin/gfortran | |
| OMPI_CC : | |
| OMPI_CXX : | |
| OMPI_FC : | |
| post_process: | |
| CMake Configuration: | |
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | |
| C : GNU v13.3.0 (/usr/bin/cc) | |
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | |
| PRE_PROCESS : OFF | |
| SIMULATION : OFF | |
| POST_PROCESS : ON | |
| SYSCHECK : OFF | |
| DOCUMENTATION : OFF | |
| ALL : OFF | |
| MPI : ON | |
| OpenACC : OFF | |
| OpenMP : OFF | |
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | |
| Doxygen : | |
| Build Type : Release | |
| Configuration Environment: | |
| CC : /usr/bin/cc | |
| CXX : /usr/bin/c++ | |
| FC : /usr/bin/gfortran | |
| OMPI_CC : | |
| OMPI_CXX : | |
| OMPI_FC : | |
| syscheck: | |
| CMake Configuration: | |
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | |
| C : GNU v13.3.0 (/usr/bin/cc) | |
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | |
| PRE_PROCESS : OFF | |
| SIMULATION : OFF | |
| POST_PROCESS : OFF | |
| SYSCHECK : ON | |
| DOCUMENTATION : OFF | |
| ALL : OFF | |
| MPI : ON | |
| OpenACC : OFF | |
| OpenMP : OFF | |
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | |
| Doxygen : | |
| Build Type : Release | |
| Configuration Environment: | |
| CC : /usr/bin/cc | |
| CXX : /usr/bin/c++ | |
| FC : /usr/bin/gfortran | |
| OMPI_CC : | |
| OMPI_CXX : | |
| OMPI_FC : | |
| CPU: | |
| CPU Info: | |
| From lscpu | |
| Architecture: x86_64 | |
| CPU op-mode(s): 32-bit, 64-bit | |
| Address sizes: 46 bits physical, 48 bits virtual | |
| Byte Order: Little Endian | |
| CPU(s): 32 | |
| On-line CPU(s) list: 0-31 | |
| Vendor ID: GenuineIntel | |
| Model name: 13th Gen Intel(R) Core(TM) i9-13900K | |
| CPU family: 6 | |
| Model: 183 | |
| Thread(s) per core: 2 | |
| Core(s) per socket: 24 | |
| Socket(s): 1 | |
| Stepping: 1 | |
| CPU(s) scaling MHz: 23% | |
| CPU max MHz: 5800.0000 | |
| CPU min MHz: 800.0000 | |
| BogoMIPS: 5990.40 | |
| Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf tsc_known_freq pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb ssbd ibrs ibpb stibp ibrs_enhanced tpr_shadow flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt clwb intel_pt sha_ni xsaveopt xsavec xgetbv1 xsaves split_lock_detect user_shstk avx_vnni dtherm ida arat pln pts hwp hwp_notify hwp_act_window hwp_epp hwp_pkg_req hfi vnmi umip pku ospke waitpkg gfni vaes vpclmulqdq rdpid movdiri movdir64b fsrm md_clear serialize pconfig arch_lbr ibt flush_l1d arch_capabilities | |
| Virtualization: VT-x | |
| L1d cache: 896 KiB (24 instances) | |
| L1i cache: 1.3 MiB (24 instances) | |
| L2 cache: 32 MiB (12 instances) | |
| L3 cache: 36 MiB (1 instance) | |
| NUMA node(s): 1 | |
| NUMA node0 CPU(s): 0-31 | |
| Vulnerability Gather data sampling: Not affected | |
| Vulnerability Ghostwrite: Not affected | |
| Vulnerability Indirect target selection: Not affected | |
| Vulnerability Itlb multihit: Not affected | |
| Vulnerability L1tf: Not affected | |
| Vulnerability Mds: Not affected | |
| Vulnerability Meltdown: Not affected | |
| Vulnerability Mmio stale data: Not affected | |
| Vulnerability Reg file data sampling: Mitigation; Clear Register File | |
| Vulnerability Retbleed: Not affected | |
| Vulnerability Spec rstack overflow: Not affected | |
| Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl | |
| Vulnerability Spectre v1: Mitigation; usercopy/swapgs barriers and __user pointer sanitization | |
| Vulnerability Spectre v2: Mitigation; Enhanced / Automatic IBRS; IBPB conditional; PBRSB-eIBRS SW sequence; BHI BHI_DIS_S | |
| Vulnerability Srbds: Not affected | |
| Vulnerability Tsa: Not affected | |
| Vulnerability Tsx async abort: Not affected | |
| Vulnerability Vmscape: Mitigation; IBPB before exit to userspace | |
| This file was created on 2026-01-29. | |
| mfc.sh: | |
| ... | |
| pre_process: | |
| CMake Configuration: | |
| CMake v3.28.3 | |
| ... | |
| Fypp : <system-path>/fypp | |
| ... | |
| CPU: | |
| CPU Info: | |
| Architecture: x86_64 | |
| ... |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
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.
| do ix = 1, 2 | ||
| do iy = 1, 2 | ||
| do iz = 1, 2 | ||
| if (((x_cc(i) - center(1, ix))**2 + (y_cc(j) - center(2, iy))**2 + (z_cc(k) - center(3, iz))**2) <= r2) then |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/common/m_ib_patches.fpp, line 657:
<comment>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.</comment>
<file context>
@@ -587,27 +622,71 @@ contains
+ do ix = 1, 2
+ do iy = 1, 2
+ do iz = 1, 2
+ if (((x_cc(i) - center(1, ix))**2 + (y_cc(j) - center(2, iy))**2 + (z_cc(k) - center(3, iz))**2) <= r2) then
+ ib_markers_sf(i, j, k) = patch_id
+ end if
</file context>
| "num_ibs must be between 1 and num_patches_max (10)") | ||
| self.prohibit(not ib and num_ibs > 0, | ||
| "num_ibs is set, but ib is not enabled") | ||
| if periodic_ibs: |
Contributor
There was a problem hiding this comment.
P2: Add a validation that periodic_ibs requires ib to be enabled; otherwise invalid configurations (periodic_ibs='T' with ib='F') will pass validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At toolchain/mfc/case_validator.py, line 360:
<comment>Add a validation that periodic_ibs requires ib to be enabled; otherwise invalid configurations (periodic_ibs='T' with ib='F') will pass validation.</comment>
<file context>
@@ -349,13 +349,25 @@ def check_ibm(self):
"num_ibs must be between 1 and num_patches_max (10)")
self.prohibit(not ib and num_ibs > 0,
"num_ibs is set, but ib is not enabled")
+ if periodic_ibs:
+ for direction in ['x', 'y', 'z']:
+ for end in ['beg', 'end']:
</file context>
Suggested change
| if periodic_ibs: | |
| self.prohibit(periodic_ibs and not ib, | |
| "periodic_ibs requires ib to be enabled") | |
| if periodic_ibs: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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.
Comment on lines
+38
to
+40
| # to fully resolve requires 40-60 cells across sphere diameter | ||
| Nx = 511 | ||
| Ny = Nx |
Contributor
There was a problem hiding this comment.
Minor doc fix: “sphere” → “circle.”
This is a 2D circle case, so the resolution comment should reference a circle.
✏️ Suggested edit
-# to fully resolve requires 40-60 cells across sphere diameter
+# to fully resolve requires 40-60 cells across circle diameter📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # to fully resolve requires 40-60 cells across sphere diameter | |
| Nx = 511 | |
| Ny = Nx | |
| # to fully resolve requires 40-60 cells across circle diameter | |
| Nx = 511 | |
| Ny = Nx |
🤖 Prompt for AI Agents
In `@examples/2D_ibm_circle_periodic/case.py` around lines 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).
Comment on lines
+38
to
+41
| # to fully resolve requires 40-60 cells across sphere diameter | ||
| Nx = 127 | ||
| Ny = Nx | ||
| Nz = Ny |
Contributor
There was a problem hiding this comment.
Align the resolution comment with the chosen grid.
With L=1 and Nx=127, the sphere diameter spans ~13 cells, not 40–60. Consider updating the comment to avoid confusion.
💡 Suggested comment tweak
-# to fully resolve requires 40-60 cells across sphere diameter
+# NOTE: fully resolving needs 40–60 cells across diameter; this example uses ~13 for faster runs🤖 Prompt for AI Agents
In `@examples/3D_ibm_sphere_periodic/case.py` around lines 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.
Comment on lines
+649
to
+666
| $:GPU_PARALLEL_LOOP(collapse=3, private='[i,j,k,ix,iy,iz]', copy='[ib_markers_sf]', & | ||
| & copyin='[patch_id,center,r2]') | ||
| do k = 0, p | ||
| do j = 0, n | ||
| do i = 0, m | ||
| do ix = 1, 2 | ||
| do iy = 1, 2 | ||
| do iz = 1, 2 | ||
| if (((x_cc(i) - center(1, ix))**2 + (y_cc(j) - center(2, iy))**2 + (z_cc(k) - center(3, iz))**2) <= r2) then | ||
| ib_markers_sf(i, j, k) = patch_id | ||
| end if | ||
| end do | ||
| end do | ||
| end do | ||
| end do | ||
| end do | ||
| end do | ||
| end do | ||
| $:END_GPU_PARALLEL_LOOP() | ||
| $:END_GPU_PARALLEL_LOOP() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what constraints exist on cylindrical coordinates
# Search for grid_geometry == 3 usage and any associated checks
rg 'grid_geometry.*==.*3|cyl_coord' src/common/m_ib_patches.fpp -B 3 -A 3Repository: MFlowCode/MFC
Length of output: 1155
🏁 Script executed:
#!/bin/bash
# Check if there are validation checks preventing periodic_ibs with cylindrical grids
rg 'periodic_ibs' src/common/m_ib_patches.fpp -B 5 -A 5Repository: MFlowCode/MFC
Length of output: 1419
🏁 Script executed:
#!/bin/bash
# Look at the non-periodic path referenced (lines 673-678) to see what it actually does
sed -n '667,689p' src/common/m_ib_patches.fppRepository: MFlowCode/MFC
Length of output: 1087
🏁 Script executed:
#!/bin/bash
# Search for any comments or documentation about cylindrical coordinate constraints
rg 'cylindrical|cyl_coord' src/common/ -A 2 -B 2 | head -100Repository: MFlowCode/MFC
Length of output: 5091
🏁 Script executed:
#!/bin/bash
# Check if num_dims is checked in relation to cylindrical coordinates
rg 'num_dims.*cyl|cyl.*num_dims|grid_geometry.*num_dims' src/common/ -B 2 -A 2Repository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check the actual current code at lines 649-666
sed -n '640,680p' src/common/m_ib_patches.fppRepository: MFlowCode/MFC
Length of output: 1988
Periodic sphere path missing cylindrical coordinate handling.
The non-periodic path (lines 673+) handles grid_geometry == 3 by converting cylindrical to Cartesian coordinates via s_convert_cylindrical_to_cartesian_coord. However, the periodic path (lines 649-666) uses x_cc, y_cc, z_cc directly in the distance calculation without this conversion.
If periodic_ibs is enabled with a cylindrical grid geometry, sphere positions would be computed incorrectly. Add the same cylindrical coordinate conversion to the periodic path, or validate that this combination is not intended.
🤖 Prompt for AI Agents
In `@src/common/m_ib_patches.fpp` around lines 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.
Comment on lines
+1895
to
+1924
| !> The goal of this subroutine is to determine the physical global domain bounds | ||
| #ifndef MFC_POST_PROCESS | ||
| impure subroutine s_mpi_global_domain_bounds | ||
| @:ALLOCATE(domain_glb(num_dims, 2)) | ||
| #ifdef MFC_MPI | ||
| call s_mpi_allreduce_min(x_domain%beg, domain_glb(1, 1)) | ||
| call s_mpi_allreduce_max(x_domain%end, domain_glb(1, 2)) | ||
| if (n > 0) then ! 2D | ||
| call s_mpi_allreduce_min(y_domain%beg, domain_glb(2, 1)) | ||
| call s_mpi_allreduce_max(y_domain%end, domain_glb(2, 2)) | ||
| if (p > 0) then ! 3D | ||
| call s_mpi_allreduce_min(z_domain%beg, domain_glb(3, 1)) | ||
| call s_mpi_allreduce_max(z_domain%end, domain_glb(3, 2)) | ||
| end if | ||
| end if | ||
| #else | ||
| domain_glb(1, 1) = x_domain%beg | ||
| domain_glb(1, 2) = x_domain%end | ||
| if (n > 0) then ! 2D | ||
| domain_glb(2, 1) = y_domain%beg | ||
| domain_glb(2, 2) = y_domain%end | ||
| if (p > 0) then ! 3D | ||
| domain_glb(3, 1) = z_domain%beg | ||
| domain_glb(3, 2) = z_domain%end | ||
| end if | ||
| end if | ||
| #endif | ||
| $:GPU_UPDATE(device='[domain_glb]') | ||
| end subroutine s_mpi_global_domain_bounds | ||
| #endif |
Contributor
There was a problem hiding this comment.
Deallocate domain_glb in finalize to avoid leaks.
domain_glb is allocated in s_mpi_global_domain_bounds but never released. Add a guarded deallocation in s_finalize_mpi_common_module to avoid memory leaks or double‑init cycles.
🛠️ Proposed fix
impure subroutine s_finalize_mpi_common_module
`#ifdef` MFC_MPI
deallocate (buff_send, buff_recv)
`#endif`
+ if (allocated(domain_glb)) then
+ @:DEALLOCATE(domain_glb)
+ end if🤖 Prompt for AI Agents
In `@src/common/m_mpi_common.fpp` around lines 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.
Comment on lines
+156
to
+231
| if (periodic_ibs) then | ||
| ! Population of Buffers in x-direction | ||
| if (bc_x%beg == BC_PERIODIC) then | ||
| $:GPU_PARALLEL_LOOP(collapse=3) | ||
| do l = 0, p | ||
| do k = 0, n | ||
| do j = 1, buff_size | ||
| ib_markers%sf(-j, k, l) = & | ||
| ib_markers%sf(m - (j - 1), k, l) | ||
| end do | ||
| end do | ||
| end do | ||
| end if | ||
|
|
||
| if (bc_x%end == BC_PERIODIC) then | ||
| $:GPU_PARALLEL_LOOP(collapse=3) | ||
| do l = 0, p | ||
| do k = 0, n | ||
| do j = 1, buff_size | ||
| ib_markers%sf(m + j, k, l) = & | ||
| ib_markers%sf(j - 1, k, l) | ||
| end do | ||
| end do | ||
| end do | ||
| end if | ||
|
|
||
| ! Population of Buffers in y-direction | ||
| if (bc_y%beg == BC_PERIODIC) then | ||
| $:GPU_PARALLEL_LOOP(collapse=3) | ||
| do l = 0, p | ||
| do k = -buff_size, m + buff_size | ||
| do j = 1, buff_size | ||
| ib_markers%sf(k, -j, l) = & | ||
| ib_markers%sf(k, n - (j - 1), l) | ||
| end do | ||
| end do | ||
| end do | ||
| end if | ||
|
|
||
| if (bc_y%end == BC_PERIODIC) then | ||
| $:GPU_PARALLEL_LOOP(collapse=3) | ||
| do l = 0, p | ||
| do k = -buff_size, m + buff_size | ||
| do j = 1, buff_size | ||
| ib_markers%sf(k, n + j, l) = & | ||
| ib_markers%sf(k, j - 1, l) | ||
| end do | ||
| end do | ||
| end do | ||
| end if | ||
|
|
||
| ! Population of Buffers in z-direction | ||
| if (bc_z%beg == BC_PERIODIC) then | ||
| $:GPU_PARALLEL_LOOP(collapse=3) | ||
| do l = -buff_size, n + buff_size | ||
| do k = -buff_size, m + buff_size | ||
| do j = 1, buff_size | ||
| ib_markers%sf(k, l, -j) = & | ||
| ib_markers%sf(k, l, p - (j - 1)) | ||
| end do | ||
| end do | ||
| end do | ||
| end if | ||
|
|
||
| if (bc_z%end == BC_PERIODIC) then | ||
| $:GPU_PARALLEL_LOOP(collapse=3) | ||
| do l = -buff_size, n + buff_size | ||
| do k = -buff_size, m + buff_size | ||
| do j = 1, buff_size | ||
| ib_markers%sf(k, l, p + j) = & | ||
| ib_markers%sf(k, l, j - 1) | ||
| end do | ||
| end do | ||
| end do | ||
| end if | ||
| end if |
Contributor
There was a problem hiding this comment.
Declare loop indices private in the periodic buffer GPU kernels.
The new $:GPU_PARALLEL_LOOP blocks omit private lists, so loop indices can be shared across threads on GPU backends. Add private='[...]' for the loop variables in each kernel.
🛠️ Proposed fix
- $:GPU_PARALLEL_LOOP(collapse=3)
+ $:GPU_PARALLEL_LOOP(collapse=3, private='[j,k,l]')
@@
- $:GPU_PARALLEL_LOOP(collapse=3)
+ $:GPU_PARALLEL_LOOP(collapse=3, private='[j,k,l]')
@@
- $:GPU_PARALLEL_LOOP(collapse=3)
+ $:GPU_PARALLEL_LOOP(collapse=3, private='[j,k,l]')
@@
- $:GPU_PARALLEL_LOOP(collapse=3)
+ $:GPU_PARALLEL_LOOP(collapse=3, private='[j,k,l]')
@@
- $:GPU_PARALLEL_LOOP(collapse=3)
+ $:GPU_PARALLEL_LOOP(collapse=3, private='[j,k,l]')
@@
- $:GPU_PARALLEL_LOOP(collapse=3)
+ $:GPU_PARALLEL_LOOP(collapse=3, private='[j,k,l]')As per coding guidelines: Declare loop-local variables with private='[...]' in GPU parallel loop macros.
🤖 Prompt for AI Agents
In `@src/simulation/m_ibm.fpp` around lines 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.
Comment on lines
+207
to
+231
| ! Population of Buffers in z-direction | ||
| if (bc_z%beg == BC_PERIODIC) then | ||
| $:GPU_PARALLEL_LOOP(collapse=3) | ||
| do l = -buff_size, n + buff_size | ||
| do k = -buff_size, m + buff_size | ||
| do j = 1, buff_size | ||
| ib_markers%sf(k, l, -j) = & | ||
| ib_markers%sf(k, l, p - (j - 1)) | ||
| end do | ||
| end do | ||
| end do | ||
| end if | ||
|
|
||
| if (bc_z%end == BC_PERIODIC) then | ||
| $:GPU_PARALLEL_LOOP(collapse=3) | ||
| do l = -buff_size, n + buff_size | ||
| do k = -buff_size, m + buff_size | ||
| do j = 1, buff_size | ||
| ib_markers%sf(k, l, p + j) = & | ||
| ib_markers%sf(k, l, j - 1) | ||
| end do | ||
| end do | ||
| end do | ||
| end if | ||
| end if |
Contributor
There was a problem hiding this comment.
Guard z‑direction periodic fill for 2D runs.
When p == 0, ib_markers%sf is allocated with only the z‑index 0; the z‑periodic loops write to negative/positive z indices and can go out of bounds if bc_z is periodic. Wrap the z‑direction block in if (p > 0) (or num_dims == 3).
🛠️ Proposed fix
- ! Population of Buffers in z-direction
- if (bc_z%beg == BC_PERIODIC) then
- $:GPU_PARALLEL_LOOP(collapse=3)
- do l = -buff_size, n + buff_size
- do k = -buff_size, m + buff_size
- do j = 1, buff_size
- ib_markers%sf(k, l, -j) = &
- ib_markers%sf(k, l, p - (j - 1))
- end do
- end do
- end do
- end if
-
- if (bc_z%end == BC_PERIODIC) then
- $:GPU_PARALLEL_LOOP(collapse=3)
- do l = -buff_size, n + buff_size
- do k = -buff_size, m + buff_size
- do j = 1, buff_size
- ib_markers%sf(k, l, p + j) = &
- ib_markers%sf(k, l, j - 1)
- end do
- end do
- end do
- end if
+ if (p > 0) then
+ ! Population of Buffers in z-direction
+ if (bc_z%beg == BC_PERIODIC) then
+ $:GPU_PARALLEL_LOOP(collapse=3)
+ do l = -buff_size, n + buff_size
+ do k = -buff_size, m + buff_size
+ do j = 1, buff_size
+ ib_markers%sf(k, l, -j) = &
+ ib_markers%sf(k, l, p - (j - 1))
+ end do
+ end do
+ end do
+ end if
+
+ if (bc_z%end == BC_PERIODIC) then
+ $:GPU_PARALLEL_LOOP(collapse=3)
+ do l = -buff_size, n + buff_size
+ do k = -buff_size, m + buff_size
+ do j = 1, buff_size
+ ib_markers%sf(k, l, p + j) = &
+ ib_markers%sf(k, l, j - 1)
+ end do
+ end do
+ end do
+ end if
+ end if📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ! Population of Buffers in z-direction | |
| if (bc_z%beg == BC_PERIODIC) then | |
| $:GPU_PARALLEL_LOOP(collapse=3) | |
| do l = -buff_size, n + buff_size | |
| do k = -buff_size, m + buff_size | |
| do j = 1, buff_size | |
| ib_markers%sf(k, l, -j) = & | |
| ib_markers%sf(k, l, p - (j - 1)) | |
| end do | |
| end do | |
| end do | |
| end if | |
| if (bc_z%end == BC_PERIODIC) then | |
| $:GPU_PARALLEL_LOOP(collapse=3) | |
| do l = -buff_size, n + buff_size | |
| do k = -buff_size, m + buff_size | |
| do j = 1, buff_size | |
| ib_markers%sf(k, l, p + j) = & | |
| ib_markers%sf(k, l, j - 1) | |
| end do | |
| end do | |
| end do | |
| end if | |
| end if | |
| if (p > 0) then | |
| ! Population of Buffers in z-direction | |
| if (bc_z%beg == BC_PERIODIC) then | |
| $:GPU_PARALLEL_LOOP(collapse=3) | |
| do l = -buff_size, n + buff_size | |
| do k = -buff_size, m + buff_size | |
| do j = 1, buff_size | |
| ib_markers%sf(k, l, -j) = & | |
| ib_markers%sf(k, l, p - (j - 1)) | |
| end do | |
| end do | |
| end do | |
| end if | |
| if (bc_z%end == BC_PERIODIC) then | |
| $:GPU_PARALLEL_LOOP(collapse=3) | |
| do l = -buff_size, n + buff_size | |
| do k = -buff_size, m + buff_size | |
| do j = 1, buff_size | |
| ib_markers%sf(k, l, p + j) = & | |
| ib_markers%sf(k, l, j - 1) | |
| end do | |
| end do | |
| end do | |
| end if | |
| end if | |
| end if |
🤖 Prompt for AI Agents
In `@src/simulation/m_ibm.fpp` around lines 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.
Comment on lines
+825
to
+831
| ! 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 |
Contributor
There was a problem hiding this comment.
Avoid domain_glb(2,*) access when num_dims==1.
domain_glb is sized (num_dims, 2). In 1D it only has row 1, but the current wrap always updates y_centroid using row 2. Guard the y/z wrapping with if (n > 0) (or num_dims > 1) to prevent out‑of‑bounds access if periodic_ibs is ever enabled in 1D.
🛠️ Proposed fix
- 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
+ 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))
+ if (n > 0) then
+ 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
+ end if🤖 Prompt for AI Agents
In `@src/simulation/m_time_steppers.fpp` around lines 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.
Comment on lines
+1
to
+193
| This file was created on 2026-01-29 18:27:29.448182. | ||
|
|
||
| mfc.sh: | ||
|
|
||
| Invocation: test --generate -f FF27BC5E -t FF27BC5E | ||
| Lock: mpi=Yes & gpu=No & debug=No & gcov=No & unified=No & single=No & mixed=No & fastmath=No | ||
| Git: 1aa4b082059aacbb5d9a0183076414923e9f6044 on master (dirty) | ||
|
|
||
| simulation: | ||
|
|
||
| CMake Configuration: | ||
|
|
||
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | ||
|
|
||
| C : GNU v13.3.0 (/usr/bin/cc) | ||
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | ||
|
|
||
| PRE_PROCESS : OFF | ||
| SIMULATION : ON | ||
| POST_PROCESS : OFF | ||
| SYSCHECK : OFF | ||
| DOCUMENTATION : OFF | ||
| ALL : OFF | ||
|
|
||
| MPI : ON | ||
| OpenACC : OFF | ||
| OpenMP : OFF | ||
|
|
||
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | ||
| Doxygen : | ||
|
|
||
| Build Type : Release | ||
|
|
||
| Configuration Environment: | ||
|
|
||
| CC : /usr/bin/cc | ||
| CXX : /usr/bin/c++ | ||
| FC : /usr/bin/gfortran | ||
| OMPI_CC : | ||
| OMPI_CXX : | ||
| OMPI_FC : | ||
|
|
||
| pre_process: | ||
|
|
||
| CMake Configuration: | ||
|
|
||
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | ||
|
|
||
| C : GNU v13.3.0 (/usr/bin/cc) | ||
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | ||
|
|
||
| PRE_PROCESS : ON | ||
| SIMULATION : OFF | ||
| POST_PROCESS : OFF | ||
| SYSCHECK : OFF | ||
| DOCUMENTATION : OFF | ||
| ALL : OFF | ||
|
|
||
| MPI : ON | ||
| OpenACC : OFF | ||
| OpenMP : OFF | ||
|
|
||
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | ||
| Doxygen : | ||
|
|
||
| Build Type : Release | ||
|
|
||
| Configuration Environment: | ||
|
|
||
| CC : /usr/bin/cc | ||
| CXX : /usr/bin/c++ | ||
| FC : /usr/bin/gfortran | ||
| OMPI_CC : | ||
| OMPI_CXX : | ||
| OMPI_FC : | ||
|
|
||
| post_process: | ||
|
|
||
| CMake Configuration: | ||
|
|
||
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | ||
|
|
||
| C : GNU v13.3.0 (/usr/bin/cc) | ||
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | ||
|
|
||
| PRE_PROCESS : OFF | ||
| SIMULATION : OFF | ||
| POST_PROCESS : ON | ||
| SYSCHECK : OFF | ||
| DOCUMENTATION : OFF | ||
| ALL : OFF | ||
|
|
||
| MPI : ON | ||
| OpenACC : OFF | ||
| OpenMP : OFF | ||
|
|
||
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | ||
| Doxygen : | ||
|
|
||
| Build Type : Release | ||
|
|
||
| Configuration Environment: | ||
|
|
||
| CC : /usr/bin/cc | ||
| CXX : /usr/bin/c++ | ||
| FC : /usr/bin/gfortran | ||
| OMPI_CC : | ||
| OMPI_CXX : | ||
| OMPI_FC : | ||
|
|
||
| syscheck: | ||
|
|
||
| CMake Configuration: | ||
|
|
||
| CMake v3.28.3 on conradd3-ThinkStation-P3-Ultra | ||
|
|
||
| C : GNU v13.3.0 (/usr/bin/cc) | ||
| Fortran : GNU v13.3.0 (/usr/bin/gfortran) | ||
|
|
||
| PRE_PROCESS : OFF | ||
| SIMULATION : OFF | ||
| POST_PROCESS : OFF | ||
| SYSCHECK : ON | ||
| DOCUMENTATION : OFF | ||
| ALL : OFF | ||
|
|
||
| MPI : ON | ||
| OpenACC : OFF | ||
| OpenMP : OFF | ||
|
|
||
| Fypp : /home/conradd3/repositories/periodic_ibs/build/venv/bin/fypp | ||
| Doxygen : | ||
|
|
||
| Build Type : Release | ||
|
|
||
| Configuration Environment: | ||
|
|
||
| CC : /usr/bin/cc | ||
| CXX : /usr/bin/c++ | ||
| FC : /usr/bin/gfortran | ||
| OMPI_CC : | ||
| OMPI_CXX : | ||
| OMPI_FC : | ||
|
|
||
| CPU: | ||
|
|
||
| CPU Info: | ||
| From lscpu | ||
| Architecture: x86_64 | ||
| CPU op-mode(s): 32-bit, 64-bit | ||
| Address sizes: 46 bits physical, 48 bits virtual | ||
| Byte Order: Little Endian | ||
| CPU(s): 32 | ||
| On-line CPU(s) list: 0-31 | ||
| Vendor ID: GenuineIntel | ||
| Model name: 13th Gen Intel(R) Core(TM) i9-13900K | ||
| CPU family: 6 | ||
| Model: 183 | ||
| Thread(s) per core: 2 | ||
| Core(s) per socket: 24 | ||
| Socket(s): 1 | ||
| Stepping: 1 | ||
| CPU(s) scaling MHz: 21% | ||
| CPU max MHz: 5800.0000 | ||
| CPU min MHz: 800.0000 | ||
| BogoMIPS: 5990.40 | ||
| Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf tsc_known_freq pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb ssbd ibrs ibpb stibp ibrs_enhanced tpr_shadow flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt clwb intel_pt sha_ni xsaveopt xsavec xgetbv1 xsaves split_lock_detect user_shstk avx_vnni dtherm ida arat pln pts hwp hwp_notify hwp_act_window hwp_epp hwp_pkg_req hfi vnmi umip pku ospke waitpkg gfni vaes vpclmulqdq rdpid movdiri movdir64b fsrm md_clear serialize pconfig arch_lbr ibt flush_l1d arch_capabilities | ||
| Virtualization: VT-x | ||
| L1d cache: 896 KiB (24 instances) | ||
| L1i cache: 1.3 MiB (24 instances) | ||
| L2 cache: 32 MiB (12 instances) | ||
| L3 cache: 36 MiB (1 instance) | ||
| NUMA node(s): 1 | ||
| NUMA node0 CPU(s): 0-31 | ||
| Vulnerability Gather data sampling: Not affected | ||
| Vulnerability Ghostwrite: Not affected | ||
| Vulnerability Indirect target selection: Not affected | ||
| Vulnerability Itlb multihit: Not affected | ||
| Vulnerability L1tf: Not affected | ||
| Vulnerability Mds: Not affected | ||
| Vulnerability Meltdown: Not affected | ||
| Vulnerability Mmio stale data: Not affected | ||
| Vulnerability Reg file data sampling: Mitigation; Clear Register File | ||
| Vulnerability Retbleed: Not affected | ||
| Vulnerability Spec rstack overflow: Not affected | ||
| Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl | ||
| Vulnerability Spectre v1: Mitigation; usercopy/swapgs barriers and __user pointer sanitization | ||
| Vulnerability Spectre v2: Mitigation; Enhanced / Automatic IBRS; IBPB conditional; PBRSB-eIBRS SW sequence; BHI BHI_DIS_S | ||
| Vulnerability Srbds: Not affected | ||
| Vulnerability Tsa: Not affected | ||
| Vulnerability Tsx async abort: Not affected | ||
| Vulnerability Vmscape: Mitigation; IBPB before exit to userspace | ||
|
|
Contributor
There was a problem hiding this comment.
Golden metadata captures a dirty git state.
The Git: ... (dirty) line will differ on clean runs and can make golden comparisons flaky. Consider regenerating from a clean tree or normalizing this field in the metadata generator.
🤖 Prompt for AI Agents
In `@tests/FF27BC5E/golden-metadata.txt` around lines 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
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
periodic_ibsparameter.Tests
✏️ Tip: You can customize this high-level summary in your review settings.