Skip to content

Host-capture Re_size in the species-to-mixture kernel and its device callers - #2195

Draft
sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:wave5/re-size-host-capture
Draft

sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:wave5/re-size-host-capture

Conversation

@sbryngelson

@sbryngelson sbryngelson commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

What

#2130 copied Re_size into firstprivate host locals for the kernels that read it from device code, because amdflang can read the declare-target module variable stale across translation units. This finishes that treatment for the remaining device read: s_convert_species_to_mixture_variables_kernel reads Re_size(i) when Re_K is present.

The kernel takes two new optional arguments Re_size_loc1, Re_size_loc2 and uses them when present; without them it reads the module variable (host callers, m_pressure_relaxation, pre/post_process). Every device caller that passes Re_K now passes firstprivate host copies, as in #2130: the two conversion kernels in m_variables_conversion, s_ibm_correct_state, s_cbc, s_compute_cell_state (now taking the copies as required arguments, with its callers in the time stepper, s_write_run_time_information, the Lagrangian particle code, and the host calls in s_report_icfl_violation), and the Lagrangian bubble loop. Values are identical to Re_size, so results do not change.

Verification

No result changes expected, none seen. Final head adda717d8.

  • AMD MI250X (Tioga, amdflang, OpenMP offload): Viscous 67, Non-Newtonian 8, IBM 71, Hypoelasticity 62, Lagrange 53 and CBC (grcbc and bc=-) 56 tests all pass.
  • AMD MI300A (Tuolumne): the same groups passed on the previous commit (before the review fixes); not rerun on the final head.
  • NVIDIA + NVHPC (A100, OpenACC): the same groups passed on the previous commit; the final head is being rerun and I will add the result as a comment.
  • Precheck passes.

Not shown: that this fixes a failure. I have no stale-read repro on current master, and the AMD suite already passed. It removes the last device read of the module variable.

This PR was made with Claude Code (AI).

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown

Claude Code Review

Head SHA: ae24cad

Files changed:

  • 7
  • src/common/m_variables_conversion.fpp
  • src/simulation/m_bubbles_EL.fpp
  • src/simulation/m_cbc.fpp
  • src/simulation/m_ibm.fpp
  • src/simulation/m_particles_EL.fpp
  • src/simulation/m_sim_helpers.fpp
  • src/simulation/m_time_steppers.fpp

Findings:
No issues found. Checked the changed hunks in all seven files. The new optional Re_size_loc1/Re_size_loc2 arguments are declared and forwarded consistently, and firstprivate is supported by GPU_PARALLEL_LOOP. Callers that don't pass the arguments (m_data_output, m_pressure_relaxation, the host wrapper) fall back to the module Re_size. I did not build or run anything, and I did not look at the unchanged m_data_output.fpp/m_pressure_relaxation.fpp callers beyond a grep. One thing to check when you run ./mfc.sh precheck: the new firstprivate line in src/simulation/m_time_steppers.fpp is about 132 characters, which may sit at the formatter's line limit.

…callers

amdflang can read the declare-target Re_size stale (as 0) from device code
in another translation unit. s_convert_species_to_mixture_variables_kernel
and s_compute_cell_state take optional host copies Re_size_loc1/2 and fall
back to Re_size when absent. The GPU kernels that pass Re set the copies
before the loop, make them firstprivate, and pass them down. Values are
identical to Re_size, so results do not change.
@sbryngelson
sbryngelson force-pushed the wave5/re-size-host-capture branch from ae24cad to adda717 Compare November 9, 2026 21:32
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/common/m_variables_conversion.fpp 977 +11
src/simulation/m_data_output.fpp 1932 +5
src/simulation/m_bubbles_EL.fpp 916 +4
src/simulation/m_ibm.fpp 1961 +4
src/simulation/m_sim_helpers.fpp 259 +4
src/simulation/m_cbc.fpp 1115 +3
src/simulation/m_particles_EL.fpp 825 +3
src/simulation/m_time_steppers.fpp 934 +3
Directory Lines Diff
common 10711 +11
simulation 30436 +26
total 49680 +37

This branch has not been deployed

No deployments
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.

1 participant