Bugfix periodic force sum

[danieljvickers]

Description

Fixes an edge case where we can cause out-of-bounds writes as we sum forces over periodic IB projections

Type of change

• Bug fix

[github-actions]

Claude Code Review

Head SHA: 390511e

Files changed:

• 2
• src/simulation/m_ib_patches.fpp
• src/simulation/m_ibm.fpp

Findings

1. [Critical] Incomplete rename — ib_idx and ib_dx are undeclared, will not compile

m_ibm.fpp:889 removes ib_idx from the local variable declaration (replacing it with encoded_ib_idx), but the loop body still references ib_idx at multiple lines that were not updated:

• patch_ib(ib_idx)%x_centroid / patch_ib(ib_idx)%y_centroid (context lines ~930–934)
• forces(ib_idx, l) / torques(ib_idx, l) (context lines ~1001–1003)
• The GPU_PARALLEL_LOOP private list at m_ibm.fpp:917 still lists ib_idx alongside the new encoded_ib_idx

All of these reference an undeclared variable and will cause a compile error.

2. [Critical] Call to non-existent subroutine s_decode_patch_id with undeclared argument ib_dx

m_ibm.fpp:926 introduces:

call s_decode_patch_id(encoded_ib_idx, ib_dx)

Neither s_decode_patch_id nor ib_dx exist anywhere in the codebase. The subroutine defined in m_ib_patches.fpp is s_decode_patch_periodicity. This call appears to be intended to decode the patch ID from the encoded marker (previously ib_idx = ib_markers%sf(i,j,k) carried the plain patch ID, now it is encoded), but the wrong subroutine name and a nonexistent output variable were used. This will fail to compile.

3. [Portability] optional dummy arguments and present() inside a $:GPU_ROUTINE

m_ib_patches.fpp makes x_periodicity, y_periodicity, z_periodicity optional in s_decode_patch_periodicity, which is decorated with $:GPU_ROUTINE(parallelism='[seq]'). The use of optional dummy arguments and the present() intrinsic inside OpenACC/OpenMP target device routines is not reliably supported across all CI-gated compilers — in particular, Cray ftn does not guarantee support for optional in !$acc routine seq contexts. This risks breaking GPU builds on Frontier/Tuolumne.

[coderabbitai]

📝 Walkthrough

Walkthrough

Two Fortran preprocessor files were modified to update immersed boundary simulation logic. In m_ib_patches.fpp, the s_decode_patch_periodicity method signature was updated to make the periodicity outputs optional; the reverse-mapping of periodicity values is now conditional on all three periodicity arguments being provided. In m_ibm.fpp, the s_compute_ib_forces GPU-parallel loop was refactored to treat the raw marker storage value as an encoded identifier that requires decoding before use in force and torque calculations, with corresponding updates to the loop's private variable declarations.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is incomplete. It lacks required sections including Testing, Checklist items, and the GPU changes section (despite modifications to src/simulation/).	Complete the PR description by adding: Testing section describing how changes were validated, Checklist items for tests/documentation, and GPU changes section confirming GPU/CPU result matching and GPU testing.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Bugfix periodic force sum' directly describes the main change: fixing a bug related to periodic force summation in the immersed boundary method, which aligns with the PR's stated objective of preventing out-of-bounds writes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/simulation/m_ibm.fpp (1)

889-926: ⚠️ Potential issue | 🔴 Critical

Critical: three compilation-blocking defects prevent the OOB fix from working.

This kernel has three defects that will cause immediate compilation failure:

1. s_decode_patch_id does not exist. The only decoder available in m_ib_patches.fpp is s_decode_patch_periodicity (defined at line 1048 with optional periodicity outputs). There is no s_decode_patch_id anywhere in the codebase.

2. ib_idx is never declared. Line 889 only declares i, j, k, l, encoded_ib_idx, fluid_idx, yet ib_idx is referenced in the GPU private=[...] clause on line 917 and used on lines 930, 931, 933, 934, 1001, and 1003.

3. ib_dx (line 926) is an undeclared typo. Should be ib_idx.

The intent is correct—to decode the encoded marker so that forces(ib_idx, :) and torques(ib_idx, :) stay within [1, num_ibs] and prevent OOB writes. However, these three bugs prevent the code from compiling and must be fixed.

🐛 Proposed fix

-        integer                                                        :: i, j, k, l, encoded_ib_idx, fluid_idx
+        integer                                                        :: i, j, k, l, encoded_ib_idx, ib_idx, fluid_idx

-                    encoded_ib_idx = ib_markers%sf(i, j, k)
-                    if (encoded_ib_idx /= 0) then
-                        call s_decode_patch_id(encoded_ib_idx, ib_dx)
+                    encoded_ib_idx = ib_markers%sf(i, j, k)
+                    if (encoded_ib_idx /= 0) then
+                        call s_decode_patch_periodicity(encoded_ib_idx, ib_idx)

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1268c24e-7202-4f84-93c1-1ab6ae7e3255

📥 Commits

Reviewing files that changed from the base of the PR and between a8a1cc5 and 390511e.

📒 Files selected for processing (2)

• src/simulation/m_ib_patches.fpp
• src/simulation/m_ibm.fpp

[codecov]

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.76%. Comparing base (f3cfba8) to head (b065912).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
src/simulation/m_ibm.fpp	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1377      +/-   ##
==========================================
+ Coverage   64.62%   64.76%   +0.14%     
==========================================
  Files          71       71              
  Lines       18407    18713     +306     
  Branches     1516     1549      +33     
==========================================
+ Hits        11895    12119     +224     
- Misses       5555     5638      +83     
+ Partials      957      956       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.