Copy the ib flag to the device - #1937
sbryngelson wants to merge 2 commits into
Conversation
MFlowCode#1792 added two device-side reads of ib -- skipping immersed-boundary interior cells in the adaptive-dt reduction (s_compute_dt) and in the CFL stability statistics (s_write_run_time_information). ib is GPU_DECLARE'd through SIM_GPU_DECL_VARS but was never GPU_UPDATE'd, so on the device it stayed .false. and GPU builds kept including IB interior cells in dt while CPU builds excluded them. With cfl_adap_dt=T this gives GPU runs a different dt from CPU runs.
|
Verified on Frontier (CCE, OpenACC GPU build, |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The targeted update correctly occurs before device kernels consume ib.
Review effort: Balanced
Findings: None
What changed in this PR
Synchronizes the immersed-boundary flag with GPU device storage, restoring CPU/GPU consistency for IB-aware CFL calculations.
Changes:
- Adds
ibto global GPU parameter initialization.
| File | Description |
|---|---|
src/simulation/m_global_parameters.fpp |
Copies ib to its existing device allocation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
2D -> 1 Fluid(s) -> IBM -> Adaptive dt: a rectangle moving through gas at rest with cfl_adap_dt. The body's interior carries its velocity, so if IB-interior cells enter the dt reduction (as they did on GPUs while ib was never copied to the device) dt shrinks ~8% and the step-end state moves by 2e-2. Edges sit on cell faces and the body moves 0.2 dx, so no cell crosses the surface and the golden is portable.
|
Re: the Copilot finding asking for a GPU regression test: added in 5cea7a0.
Checked on Frontier (CCE), with the golden generated on CPU from this branch:
So removing the update again turns this test red. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1937 +/- ##
==========================================
- Coverage 62.82% 62.80% -0.03%
==========================================
Files 86 86
Lines 22394 22387 -7
Branches 3305 3306 +1
==========================================
- Hits 14070 14061 -9
Misses 6071 6071
- Partials 2253 2255 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
ibgets a device copy (GPU_DECLARE(create=...)viaSIM_GPU_DECL_VARS), but nothing ever copies its value to the GPU, so inside device kernels it stays.false.. #1792 added two device-side reads of it:s_compute_dt(m_time_steppers.fpp): skip IB-interior cells in the adaptive-dt reductions_write_run_time_information(m_data_output.fpp): skip IB-interior cells in the CFL stability statisticsOn GPU builds both checks are silently skipped. GPUs keep including IB-interior cells when choosing dt, while CPU builds exclude them. Any case with
ibandcfl_adap_dt = Tthen runs with a different dt on GPU than on CPU. This addsibto the existingGPU_UPDATE(device=...)ins_initialize_global_parameters_module.How it showed up
In #1821, the
2D -> Example -> ibm_reacting_surfacetest (F52F0D4C) gives the same wrong numbers on every GPU build (Frontier CCE-acc, CCE-omp, AMD-omp, and Phoenix NVHPC-acc: 0.65037 vs. golden 0.6487849), while every CPU build (GNU, CCE, AMD) matches. Before the merge of #1792 into that branch, CPU and GPU agreed. After it, GPU stayed on the old value and only CPU moved. No master test currently triggers the bug.The other device-declared parameters that are never updated (
down_sample,igr_pres_lim,recon_type,weno_order) are case-optimization parameters that are read on the host or baked in at compile time, so this PR leaves them alone.This PR was prepared with Claude Code (diagnosis from the #1821 CI logs and the fix).
Acknowledgement