Repository navigation
IBM flux wall: positivity-factor kernel without per-thread arrays - #2216
Open
sbryngelson wants to merge 1 commit into
Open
sbryngelson wants to merge 1 commit into
sbryngelson wants to merge 1 commit into
Conversation
f_wall_face_sum built per-thread arrays h(3) and c(3). It now sums the neighbour terms through f_wall_face_term, so only the order of the floating-point additions changes. MI300A (amdflang AFAR, OpenMP offload), reacting cylinder with ib_flux_wall, 96 cells/D, 100 steps, 3 interleaved repeats: s_ib_flux_wall_prepare took 1.909 ms per call (34% of kernel time) before and 0.042 ms after (rocprofv3). Step cost, ns/gp/eq/rhs, mean of 3: option off 11.27, on 15.46 before and 12.39 after (-17 to -23% per repeat). A100 (nvfortran 25.11, OpenACC): on 6.02 before, 5.94 after, within noise. IBM (85), Chemistry (37) and Flux Wall tests pass on MI300A and A100 with unchanged goldens.
Lines of Code
|
|
Claude Code Review Head SHA: 7ca55a5 Files changed:
Findings: |
sbryngelson
marked this pull request as ready for review
November 13, 2026 08:21
sbryngelson
requested review from
danieljvickers
and removed request for
a team
November 13, 2026 08:21
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on
origin/master80997da. One function changes (f_wall_face_suminsrc/simulation/m_ibm.fpp); no parameter, default, or output changes.Summary
f_wall_face_sum(used bys_ib_flux_wall_prepare, which runs three times per step, and bys_ib_flux_wall_setup) built per-thread arraysh(3)andc(3)and indexed the neighbour cells through them. It now sums the four (six in 3D) neighbour terms directly through a small helper,f_wall_face_term. Only the order of the floating-point additions changes.On the MI300A (amdflang, OpenMP offload) the old kernel is very slow:
s_ib_flux_wall_preparetakes 34% of all GPU kernel time of a reactingib_flux_wallcase. The cause is assumed to be that the compiler places the per-thread arrays in scratch (private) memory; I did not inspect the generated code or register/scratch usage, I only measured that removing the arrays removes the cost. On an A100 (nvfortran, OpenACC) the old kernel was not slow and the change is within noise.Evidence
Case:
examples/2D_ibm_reacting_surfacephysics (reduced GRI mechanism, carbon surface, reacting cylinder withsurface_reaction = 1,thermal_bc = 1) as a periodic square array of one cylinder, area fraction 0.2, body-force driven, 96 cells per diameter (190 x 190 cells), 100 fixed steps, 1 rank,ib_flux_wall = T("on") or default ("off"). Metric:Performance(ns/gp/eq/rhs) printed withrun_time_info = T. Three repeats, interleaved (off, on-master, on-PR, off, ...) on the same node in one job. "master" is a build of the base commit; "PR" is this change.MI300A (AMD MI300A, amdflang AFAR,
--gpu mp), ns/gp/eq/rhs:The cost of the option over "off" falls from +37% to +10% (means). All three numbers drift upward from repeat 1 to 3 on this node, which is why the comparison is within each interleaved repeat.
rocprofv3 kernel trace (
--kernel-trace --stats, same case, 100 steps, 300 calls of the kernel):s_ib_flux_wall_prepare, ms per calls_ib_flux_wall_prepare, total msA100 (NVIDIA A100, nvfortran 25.11, OpenACC,
--gpu acc, idle GPU), ns/gp/eq/rhs:About 1% (the repeat-to-repeat spread of "off" is 9%), so I make no performance claim for NVIDIA. No profile was taken on the A100.
Tests
Existing goldens, no regeneration;
./mfc.sh test --only <label> --no-buildafter a--dry-runbuild of each mechanism.Flux Wall(C1F0BCB1, the only test withib_flux_wall)--gpu mp)--gpu acc)No golden moved.
./mfc.sh formatand./mfc.sh precheckpass.Limitations
s_ib_wall_src_buffers, whose per-call time varied between 0.038 and 0.067 ms between the two profiled runs) were not studied.ib_flux_wall = Tcan differ at round-off level from master; the existing test passes within its tolerance on both vendors.ib_flux_wallconfigurations beyond the one test above.This PR was made with Claude Code (AI).