fix(sim): Euler–Euler bubbles GPU data race on shared bubble-wall scratch#1648
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a GPU data race in the Euler–Euler bubble source path by removing module-scope bubble-wall scratch scalars and instead threading those intermediates through the bubble-wall/vapor-flux routines as per-thread values, avoiding cross-thread clobbering on device.
Changes:
- Removed module-scope
chi_vw,k_mw, andrho_mwscratch state fromm_bubbles. - Extended
s_vfluxandf_bpres_dotto accept per-thread bubble-wall intermediates (via optional arguments) for the Euler–Euler path. - Updated the Euler–Euler source loop to allocate/privatize per-thread scratch (
chi_vw_l,k_mw_l,rho_mw_l) and pass it through the call chain inside the GPU parallel region.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/simulation/m_bubbles.fpp | Removes shared module scratch and threads bubble-wall intermediates into s_vflux/f_bpres_dot. |
| src/simulation/m_bubbles_EE.fpp | Uses per-thread scratch within the GPU loop and passes it into the updated bubble-wall routines to avoid races. |
| @@ -287,8 +283,8 @@ contains | |||
| end if | |||
| else | |||
| chi_bar = fmass_v/(fmass_v + mass_g0(iR0)) | |||
| grad_chi = -Re_trans_c(iR0)*(chi_bar - chi_vw) | |||
| vflux = rho_mw*grad_chi/Pe_c/(1._wp - chi_vw)/fR | |||
| grad_chi = -Re_trans_c(iR0)*(chi_bar - fchi_vw) | |||
| vflux = frho_mw*grad_chi/Pe_c/(1._wp - fchi_vw)/fR | |||
There was a problem hiding this comment.
The thermal == 3 .and. .not. bubbles_lagrange branch that reads fchi_vw/frho_mw is only reachable from the Euler–Euler source loop (m_bubbles_EE), which always computes these via s_bwproperty into per-thread scratch and passes them (fchi_vw=chi_vw_l, frho_mw=rho_mw_l). The other callers — the Lagrangian path (m_bubbles_EL) and the adaptive substep — run with bubbles_lagrange = .true., so they take the bubbles_lagrange branch above and never touch these optionals. So the branch is never entered with the optionals absent.
These are $:GPU_ROUTINE(parallelism='[seq]') device routines, so an @:ASSERT(present(...)) guard isn't available on-device, and a local s_bwproperty fallback would duplicate the bubble-wall computation into the device path for an unreachable case; the coupling is enforced by the caller structure instead. (PR is now rebased onto master post-#1290, which reworked these routines to non-elemental — the fix re-applies cleanly.)
| @@ -323,7 +320,7 @@ contains | |||
| end if | |||
| grad_T = -Re_trans_T(iR0)*((fpb/pb0(iR0))*(fR/R0(iR0))**3*(mass_g0(iR0) + mass_v0(iR0))/(mass_g0(iR0) + fmass_v) & | |||
| & - 1._wp) | |||
| f_bpres_dot = 3._wp*gam_m*(-fV*fpb + fvflux*R_v*Tw + pb0(iR0)*k_mw*grad_T/Pe_T(iR0)/fR)/fR | |||
| f_bpres_dot = 3._wp*gam_m*(-fV*fpb + fvflux*R_v*Tw + pb0(iR0)*fk_mw*grad_T/Pe_T(iR0)/fR)/fR | |||
| else | |||
There was a problem hiding this comment.
Same reasoning as the s_vflux thread: the thermal == 3 non-Lagrangian path that uses fk_mw is only reached from the Euler–Euler loop (m_bubbles_EE), which passes fk_mw=k_mw_l; the Lagrangian/adaptive callers take the bubbles_lagrange branch (which returns before fk_mw is used). Device $:GPU_ROUTINE, so no on-device assert; coupling enforced by the callers.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1648 +/- ##
=========================================
Coverage ? 59.57%
=========================================
Files ? 83
Lines ? 21111
Branches ? 3128
=========================================
Hits ? 12576
Misses ? 6433
Partials ? 2102 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
chi_vw/k_mw/rho_mw were module-scope GPU_DECLARE'd scalars used as per-cell scratch inside the s_compute_bubble_EE_source parallel loop (s_bwproperty writes them; s_vflux/f_bpres_dot read them via host association) but were not private, so GPU threads clobbered each other's values between write and read - a scheduling-dependent error (amdflang 3.6e-7, CCE-OMP 1.6e-9) on the nonpolytropic partial pressures. Thread them through arguments instead (fchi_vw/frho_mw to s_vflux, fk_mw to f_bpres_dot) with per-thread locals, and delete the module scalars. Validated: 454C565F now passes at its 5e-9 tol on amdflang.
673ad8e to
288a3d7
Compare
…ga/amr-scaling Resolves 11 textual conflicts (bubble sigs=MFlowCode#1648 replay, m_ibm=AMR global ghost_points + MFlowCode#1290 glb_bounds, rest additive) plus 2 semantic conflicts git auto-merged silently: AMR's s_lag_cloud_bbox_local nBubs->n_el_bubs_loc (MFlowCode#1290 moved the local bubble count to m_global_parameters) and s_lag_phys_to_cells/s_amr_body_bbox x/y/z_domain->glb_bounds (MFlowCode#1290 removed x/y/z_domain from sim, replaced w/ grid-derived glb_bounds). CPU-validated: 50/50 pure-AMR + 26/26 non-AMR Lagrange byte-identical; 2 AMR+Lag goldens (4B08E9B7,BCE1BBAE) regenerated for MFlowCode#1290's new EL physics. GPU np>=2 pending.
Fixes a GPU data race in the Euler–Euler bubble source. The bubble-wall intermediates
chi_vw,k_mw, andrho_mwwere module-scope scalars written and read by every GPU thread in the bubble-dynamics loop, so concurrent threads clobbered each other's values (a nondeterministic race on the shared scratch). They are now threaded as per-thread (private) arguments through the bubble-wall routines instead of shared module state.Affects non-polytropic Euler–Euler bubble runs.
Extracted as a standalone fix from the AMR/scaling work (
up/mega) since it is independent of AMR. CI runs the full compiler × GPU matrix as the gate.