Skip to content

Make the IB force-reduction receive buffers device resident - #1843

Open
sbryngelson wants to merge 5 commits into
masterfrom
fix/ib-force-reduction-offload
Open

Make the IB force-reduction receive buffers device resident#1843
sbryngelson wants to merge 5 commits into
masterfrom
fix/ib-force-reduction-offload

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

Fixes #1840.

s_communicate_ib_forces hands recv_forces_snap, recv_torques_snap, recv_ids and recv_ft to GPU kernels via copy/copyin, but they are allocated with a plain allocate and so are never present on the device. Under Cray OpenMP offload this aborts at the first time step for any moving immersed boundary run on more than one rank:

ACC: find_in_present_table failed for 'recv_forces_snap(:,:)' from .../m_ibm.fpp:1338
ACC: libcrayacc/acc_runtime.c:703 CRAY_ACC_ERROR - Variable not found in present table
srun: error: frontier10177: tasks 1-3: Exited with exit code 1

The send-side twins (send_ids, send_ft) are already @:ALLOCATEd and pushed with GPU_UPDATE, so this only makes the receive side consistent with them: @:ALLOCATE/@:DEALLOCATE, an explicit GPU_UPDATE(device=...) after each host write, and the corresponding copy/copyin clauses dropped.

Testing. Frontier, --gpu mp (cpe/25.03, rocm/6.3.1), 2D moving flat plate with ib_state_wrt, 4 ranks: aborts at step 1 on master, runs to completion with this patch. An OpenACC build of the same case was unaffected before and after. ./mfc.sh precheck passes.

One file, 8 insertions, 6 deletions.

https://claude.ai/code/session_01HMJ7cycfo7kTFSFq5yhHLG

`s_communicate_ib_forces` passed `recv_forces_snap`, `recv_torques_snap`, `recv_ids` and
`recv_ft` into GPU kernels through `copy`/`copyin`, but allocated them with a plain
`allocate`, so they never entered the device present table. On Cray OpenMP offload this
aborts at the first time step of any moving-IB case run on more than one rank:

    ACC: find_in_present_table failed for 'recv_forces_snap(:,:)' from m_ibm.fpp:1338
    ACC: libcrayacc/acc_runtime.c:703 CRAY_ACC_ERROR - Variable not found in present table

The sibling send buffers `send_ids`/`send_ft` are already `@:ALLOCATE`d and pushed with
`GPU_UPDATE`, so the receive side was simply inconsistent with them. Allocate the four
receive arrays the same way and update them to the device after the host writes (the
zeroing before each accumulation pass, and each `MPI_UNPACK`), which also lets the kernels
drop the `copy`/`copyin` of those arrays.

Reproduced on Frontier (`./mfc.sh build --gpu mp`, cpe/25.03, rocm/6.3.1) with a 2D moving
flat plate on 4 ranks; the same case runs to completion with this change, and an OpenACC
build was unaffected either way.

Fixes #1840

Claude-Session: https://claude.ai/code/session_01HMJ7cycfo7kTFSFq5yhHLG
Copilot AI lite review requested due to automatic review settings September 11, 2026 14:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

This PR fixes a GPU offload crash in multi-rank immersed boundary runs by ensuring receive-side force/torque buffers are device-resident and explicitly synchronized for OpenMP offload runtimes (e.g., Cray).

Changes:

  • Switch receive buffers to @:ALLOCATE/@:DEALLOCATE so they become device-resident like the send-side twins.
  • Add GPU_UPDATE(device=...) after host writes to recv_* receive buffers.
  • Remove copy/copyin clauses for receive buffers in GPU parallel loops to avoid present-table failures.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/simulation/m_ibm.fpp Outdated
Comment on lines +1339 to +1340
$:GPU_UPDATE(device='[recv_ids, recv_ft]')
$:GPU_PARALLEL_LOOP(private='[i, j]', copy='[forces, torques]')
Comment thread src/simulation/m_ibm.fpp Outdated
call MPI_UNPACK(ib_force_recv_buf, buf_size, unpack_pos, recv_ft, 6*recv_count, mpi_p, MPI_COMM_WORLD, ierr)
$:GPU_PARALLEL_LOOP(private='[i, j]', copyin='[recv_ft, recv_ids]', copy='[forces, torques, &
& recv_forces_snap, recv_torques_snap]')
$:GPU_UPDATE(device='[recv_ids, recv_ft]')
Comment thread src/simulation/m_ibm.fpp Outdated
Comment on lines +1385 to +1386
$:GPU_UPDATE(device='[recv_ids, recv_ft]')
$:GPU_PARALLEL_LOOP(private='[i, j]', copy='[forces, torques]')
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.26%. Comparing base (e7139bc) to head (99df96c).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
src/simulation/m_ibm.fpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1843   +/-   ##
=======================================
  Coverage   61.26%   61.26%           
=======================================
  Files          84       84           
  Lines       22330    22329    -1     
  Branches     3265     3265           
=======================================
  Hits        13680    13680           
+ Misses       6207     6206    -1     
  Partials     2443     2443           

☔ View full report in Codecov by Harness.
📢 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.

@sbryngelson

Copy link
Copy Markdown
Member Author

Worked through the three Copilot comments.

The two full-array update comments are right and are fixed in 29034b0e0. size(patch_ib) is num_local_ibs_max = 2000, so each GPU_UPDATE pushed about 104 KB (recv_ft at 6x2000x8 B plus recv_ids at 8 KB) no matter how many bodies actually arrived, once per neighbour per direction per step. Both sites now update what was written:

$:GPU_UPDATE(device='[recv_ids(1:recv_count), recv_ft(:,1:recv_count)]')

Safe because the kernel below only reads 1:recv_count; the untouched tail is never referenced.

I did not add the suggested recv_count == 0 guard. The loop is already a no-op at zero, a zero-length section is legal, and the surrounding code does not guard its own slices (GPU_UPDATE(host='[patch_ib(1:num_ibs)]')), so the branch would only be there for a case that cannot misbehave.

The recv_forces_snap / recv_torques_snap comment does not apply. It is conditional on host code reading those arrays after the kernel, and nothing does: they are module-private, appear nowhere outside m_ibm.fpp, and their only accesses are the host zeroing plus device update at 1311-1313 and the read/write inside the kernel at 1345-1348. There is no host read between the kernel and the @:DEALLOCATE. Keeping them device resident is the point - the running snapshot now accumulates across receive iterations instead of round-tripping through the host on every launch. Adding a GPU_UPDATE(host=...) would just reintroduce the transfer the PR removes.

Verified on AFBACA70 (3D -> 2 MPI Ranks -> IBM Sphere) plus CE232828, A2036630 and 16574715, amdflang OpenMP offload on MI210: 4 passed, 0 failed. AFBACA70 is the case that matters - this path only runs when num_procs > 1, so the single-rank IB tests never touch it.

b2ef85321 is unrelated: master's Lint Toolchain has been red since #1855 and this branch has that merge, so the three doc-reference fixes are here to get the lint gate green. Same change as #1861; it will reconcile when that lands.

@github-actions

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/simulation/m_ibm.fpp 1255 +2
Directory Lines Diff
simulation 28088 +2
total 46342 +2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Moving IB + MPI on Cray OpenMP offload: recv_forces_snap not in present table (s_communicate_ib_forces)

2 participants