Skip to content

Euler-Lagrange capabilities - Phase 1 - #1926

Draft
thierrydaoud wants to merge 68 commits into
MFlowCode:masterfrom
thierrydaoud:el-phase1
Draft

thierrydaoud wants to merge 68 commits into
MFlowCode:masterfrom
thierrydaoud:el-phase1

Conversation

@thierrydaoud

@thierrydaoud thierrydaoud commented Sep 28, 2026 •

Copy link
Copy Markdown

Summary

This PR adds a Lagrangian solid-particle model (particles_lagrange) next to the existing Lagrangian bubble model. Rigid spherical particles are tracked individually. The gas acts on them through quasi-steady drag (Gidaspow, Parmar et al., or Osnes et al.), optional pressure-gradient and added-mass forces, and an optional stochastic drag-fluctuation force. With two-way coupling, the particles act back on the gas through momentum and energy source terms, projected onto the grid with the Gaussian kernel of Maeda & Colonius (2018).

The solver is ported from our research fork (MFC-EL) and adapted to current master (generated parameters, m_eos, the refactored Riemann-state reconstruction). Phase 1 has no particle–particle collisions.

Existing cases are unaffected, with one exception: periodic_bc is now broadcast to every rank (on master only rank 0 set it), which changes multi-rank periodic bubbles_lagrange results. Every other non-particle code path, including the Lagrangian bubble model, behaves as before; no existing golden file is modified.

Usage and Applications

This code capability is essential to simulate complex flows in the Euler-Lagrange framework. Problems such as a hemi-sphere blast, a planar particle curtain shock, and many other were tested and can be simulated using the developed capability.

What's included

  • Solver: src/simulation/m_particles_EL.fpp (driver, dynamics, sources, RK update, boundaries, MPI handover, I/O) and src/simulation/m_particles_EL_kernels.fpp (GPU seq kernels: Gaussian projection, interpolation, force model, drag correlations). The module is private by default and exports only what the time stepper, RHS and start-up need. Per-particle arrays are sized by the rank's own particle count plus 50 % spare and grow during the MPI handover, instead of nparticles_glb on every rank.
  • Inputs: particles_lagrange, particle_pp%… (physical properties), particle_params%… (solver settings) and lag_voidfrac_wrt. They are registered in toolchain/mfc/params/definitions.py, so the namelists, broadcasts and declarations are generated. coupling takes "one_way" / "two_way" for both bubbles and particles; the old lag_params%solver_approach is reported as renamed, naming its replacement.
  • Validation: check_particles_lagrange in case_validator.py, with 37 unit tests. It rejects the combinations the particle path does not support (see Known limitations), requires pressure_gradient_force with two-way coupling, and requires rho0ref_particle > 0.
  • Docs: particle section and parameter table in docs/documentation/case.md.
  • Examples: examples/2D_particle_hemisphere (Mach 10 blast through a half-ring of particles) and examples/2D_particle_curtain (planar shock through a dense particle curtain between slip walls). Both generate their particles in case.py.
  • Regression tests: 28 golden tests (alter_particles in cases.py, plus the two examples). 2D and 3D: one-way, tracer, two-way, no added mass, drag fluctuations, and on 2 ranks one-way, two-way, drag fluctuations, fd_order = 4 and interpolation_order = 6. 2D only: Gidaspow, Parmar, a quiescent cloud (gas must stay at rest), a closed slip-wall box, particles pressed against two walls with cells at the void limit, and a restart roundtrip. The goldens include every particle's position, velocity and force at every step.

Changes to shared code

These are the parts reviewers of other models may want to look at. Bubble call sites keep their previous behavior unless stated.

  • Halo width (m_helper_basic.fpp): s_configure_coordinate_bounds takes particles_lagrange alongside bubbles_lagrange, and fd_number is set for particles.
  • Smoothed-field (beta) exchange: s_populate_beta_buffers takes the list of fields to exchange (bubbles pass their 2–3, particles up to 11). The halo buffer (m_mpi_common.fpp) is sized from the largest field count of the active model (num_beta_vars_in; 0 in pre/post-process) instead of a fixed 11, with an abort if a call exceeds it.
  • Wall boundaries for the beta fields: for particles, slip and no-slip walls now fold the kernel's ghost-cell share back into the domain, as reflective boundaries already did. Before, up to 28 % of a wall-touching particle's volume, momentum and energy deposit was dropped. Bubbles are unchanged.
  • MPI handover (m_mpi_proxy.fpp): solid-particle send/receive. The allocation and count exchange shared by bubbles and particles are s_allocate_lag_entity_comm and s_exchange_lag_entity_counts; the buffers grow on demand.
  • periodic_bc broadcast (m_mpi_proxy.fpp): now broadcast to all ranks. This is a fix (on master only rank 0 set it), but it changes multi-rank periodic bubbles_lagrange results.
  • Lagrangian restart file: mpiiofs and mpi_info_int (MPI_INFO_NULL without parallel I/O) are now set in both I/O modes, and the shared reader no longer requires parallel_io, so particle restarts work with serial I/O. The bubble writer keeps its parallel_io gate, and the validator now rejects bubble restarts without parallel_io (previously a run-time abort).
  • Saved Riemann states (m_rhs.fpp): each direction's reconstructed states are copied at the end of s_reconstruct_riemann_states for the particle gradient fields.
  • Parallel-I/O slot for the volume-fraction field (MPI_IO_DATA sized sys_size + 1), and a t = 0 save for particle runs so post-process sees the real volume fraction.
  • Post-process: one lag_name replaces 14 hard-coded lag_bubbles names in the text and Silo writers. The Silo calls now pass the real name length (one existing call passed 16 for an 11-character name).
  • Toolchain: unknown parameters are reported with their suggestions before the JSON-schema check (they used to stop at a bare schema error). In the test harness, the --test-all template keeps fd_order for 3D particle cases (it set fd_order = 1, which the validator rejects, so post-process never ran for them), and restart tests of Lagrangian particles are skipped without MPI.
  • Misc: s_prng_splitmix32 in m_helper.fpp (the drag-fluctuation generator: exact integer arithmetic, so the stream is compiler-independent), _emit_struct_bcast in the Fortran generator, a packer rule so lag_particle output files skip their header line like lag_bubble files, and the bubble restart IDs are written at working precision (they collided above 2^24).

Verification

After the October 24 review (macOS, GNU 16.2, CPU + MPI):

Check Result
Lagrangian regression tests (particles, bubbles, examples) 56/56 pass
Restart roundtrip, 2D two-way with drag fluctuations, serial I/O restarted run matches the straight run bitwise (flow, beta, particle evolution); fails if the seed is not restored
fd_order = 4 and interpolation_order = 6, 1 vs 2 ranks (2D and 3D) agree to 5e-13 – 2e-12 of the field maximum, as the fd_order = 2 2-rank tests
Wall-layer test vs. each of the three wall/void-limit fixes reverted fails for each (rel 9e-6, 4e-3, 3e-4)
Kernel deposit kept in the domain, slip wall mid-domain, wall-adjacent, at y = r_p and in a corner: 1.00000 (was 0.917 and 0.720 at the wall)
Toolchain unit tests; ./mfc.sh validate on every example pass
./mfc.sh precheck on HiPerGator (Linux) pass (at 09b8873b)

Before the October 24 review (not rerun since; GPU runs on LLNL Tuolumne, Cray CCE, OpenACC):

Check Result
Particle regression tests, GPU, against the CPU goldens 8/8 pass (tolerance 1e-10)
2D Mach 10 hemisphere, 1612 particles, two-way with pressure-gradient and added-mass forces: GPU vs CPU flow ≤ 5e-9 relative; particle positions ≤ 5e-18 m, velocities ≤ 1e-13 relative
Same case, full 1009-step run: HiPerGator 8 ranks (GCC 14) vs Mac 1 rank (GCC 15) flow ≤ 1.4e-8 relative, particle positions ≤ 2e-13 m at every save
Rank independence, hemisphere 1 vs 3 ranks bit-for-bit; 8 ranks (cloud split across two ranks) 5e-9
Debug vs release build (CPU); debug build with bounds checking, multi-rank agree to 1e-13 (particles) and 1e-9 (flow); clean
Weak scaling, particle memory per GPU with local-count arrays (Tuolumne) −25 to −28 % on 2 GPUs, −54 % on 4

Bugs found and fixed during verification

Running the GPU and debug builds against the CPU reference turned up several bugs, all fixed in this branch:

  1. Gradient weights computed on the GPU with run-time-sized private arrays. In two-way coupling the GPU gave particles about 5 % too slow. The weights are now computed once on the host and copied to the device.
  2. Unset z-component of the fluid velocity in 2D. It entered the added-mass force through a 3-component dot product: zero by luck in CPU release builds, NaN in debug builds (which silently dropped the added-mass force), something else on GPU.
  3. Zero-slip drag. The drag correlations are singular at zero slip; the resulting NaN was caught by a finiteness check that zeroed the whole particle force, including the pressure-gradient and added-mass parts. Drag is now skipped when the slip is zero.
  4. Halo buffer overflow in the multi-rank beta exchange with qs_fluct_force (found with bounds checking).
  5. GPU loop hygiene: four unclosed GPU_PARALLEL_LOOPs, and two scalars missing from private lists.
  6. Particle output on GPU was written from stale host copies; the state is now updated from the device first, in a format that cannot overflow.
  7. Parallel-I/O crash at the first save (MPI_IO_DATA too small for the volume-fraction slot).

From the October 24 review: cells clamped to the void limit received no sources; kernel deposits in wall ghost cells were dropped; particles were clamped verysmall (not their radius) inside a wall, which in single precision left them out of bounds; restart IDs went through default-kind real(); a nonzero w in a 2D input heated the gas every step.

Known limitations

These are features the phase-1 solver does not support yet. None of them is a regression, and the validator rejects the unsupported combinations.

  • Periodic and reflective boundaries: rejected. Particle wrapping is untested. This is the next PR.
  • Collisions are not included. The particle_pp%*_col constants are accepted but unused until then.
  • Not supported with particles: cyl_coord, bubbles_lagrange, bubbles_euler, ib, igr, body forces (the particles get no gravity), state-dependent equations of state (the particle forces use the stiffened-gas gamma/pi_inf), one-way coupling with added mass, and the bubble-only lag_*_wrt outputs.
  • Restarts need the same number of ranks as the run that wrote the file, and an MPI build (the particle restart file is written with MPI-IO). With added_mass_force = 1 the first stage after a restart uses zero fluid acceleration, which is not saved. lag_db_wrt still needs parallel_io = T.
  • Performance has not been profiled. The halo exchange uses the bubble mapCells = 3, although the particle kernel only reaches one cell.

Notes for reviewers

  • The branch merged master at deb640eb (master 961cde5e).
  • Commits were made with --no-verify on macOS, where four upstream toolchain tests (test_monitor_*, test_bench_preflight, test_submit_requeue) hang or fail without these changes too. Every precheck step was run manually for each commit, and the full precheck passes on Linux.
  • In --test-all, the post-process run's return code is not checked (toolchain/mfc/test/test.py), so a failing post-process run goes unnoticed; this is how the 3D particle fd_order problem above stayed hidden. Checking it may expose failures elsewhere in the suite, so it is left for a separate PR.

@sbryngelson
sbryngelson marked this pull request as draft September 29, 2026 02:52

@sbryngelson sbryngelson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this. It is a large, carefully tested port, and the verification table and the list of bugs found along the way are genuinely useful. I reviewed CI, the toolchain, the tests and the examples myself, and read the two new Fortran modules and the shared-code changes in full. Several issues need fixing before this can merge; inline comments carry the specifics. Nothing below was built or run locally except where stated; the CI observations come from the job logs of the current run.

Commit attribution. Please rewrite the branch so the commits carry no Co-Authored-By: Claude ... <noreply@anthropic.com> trailers (12 of the 13 non-merge commits have one), and drop the "Generated with Claude Code" footer from the description. We do not list AI tools as MFC contributors. The AI disclosure paragraph is fine to keep. While you are at it, the commits are authored as t.daoud <TDAOUD@MAE-RYX75DJ.local>, a machine-local address, so GitHub does not credit them to your account; squashing into commits authored with your GitHub-linked email fixes both.

CI: 38 failing jobs, three causes

  1. Both new examples fail case validation on every lane: fluid_pp(1)%eos = 'ideal_gas' has no stiffness; do not set fluid_pp(1)%pi_inf. That rule was already in the master merged into this branch, so the examples were not rerun after the merge. They also have no goldens (tests 242025F6, 45F842CA) and are not in the Example skip list, so they will fail again once they validate.
  2. 2D -> Lagrange Particles -> Two-way Coupling -> qs_fluct_force (EDD3540A) fails on NVHPC CPU (e.g. 23.11, 26.3) with a relative error of 8e3 in beta.6. See the inline notes on the seed overflow and the per-stage OU update; either would make this non-portable.
  3. The NVHPC GPU builds crash the compiler on m_particles_EL.fpp (fort2 TERMINATED by signal 11, on 26.3 OpenACC and 25.5 OpenMP on Phoenix). The GPU verification in the description was done with CCE only; NVHPC is our main GPU compiler, so this has to build there.

Blockers (physics and correctness)

  1. Momentum is not conserved in two-way coupling: the source weights are normalized by sum(G V alpha_f) and the RHS divides by alpha_f again, so the fluid receives about F/<alpha_f> for a particle force F (inline).
  2. Energy is not conserved: SE is always zero and the fluid energy source is S.u_f, so the drag dissipation beta|u_f - u_p|^2 disappears instead of heating the gas (inline).
  3. A quiescent cloud (uniform p, u = 0, fixed particles, nonuniform alpha_f) is not an equilibrium: the -(p/alpha_f) grad(alpha_f) momentum term is unbalanced (inline).
  4. fd_order unset or 1 is accepted for particle runs and gives a negative or empty gradient stencil (inline).
  5. One-way coupling with added mass runs with du/dt = 0 because rhs_old is filled only in the two-way path. This is listed under known limitations; the validator should reject the combination rather than run it (inline).
  6. The new t = 0 save also runs on restarts and rewrites the checkpoint being restarted from (inline).
  7. Restarts assign particles by rank index (part_id = proc_particle_counts(proc_rank + 1)), so a restart on a different rank count reads out of range or loses particles, and particle_seed/fqs_fluct are not saved. Either support it or abort when the rank count changes.

Should fix

  • The drag correlation details flagged inline (Ma and Ahmadi coefficient, Osnes Mach floor, undocumented Loth modifications, stochastic force advanced every RK stage).
  • Non-finite forces are silently zeroed (kernels ~648-667), and interpolated rho/p/alpha_f are unbounded near shocks. Please count or abort instead of hiding them.
  • Cost and memory: six full sys_size arrays of reconstructed states are allocated and copied every stage (inline); particle arrays and p_send_ids are sized by nparticles_glb on every rank; the particle state moves between host and device several times per stage. None of this is a problem at 20 particles; all of it is at 1e6.
  • lag_voidfrac_wrt is not restricted to particle runs; on a bubble run it writes the bubble rvel column labelled as void fraction.
  • cyl_coord has a partial code path but no 1/r terms or velocity conversion; please prohibit it in Phase 1.
  • Two small behavior changes to non-particle runs should be mentioned in the description: the nt <= 1 timing fix, and the bubble Silo multimesh name length (16 -> 11).

Cleanliness

  • Registered but never read: particle_pp%cp_particle, ksp_col, nu_col, E_col, cor_col (the tests even set cp_particle). Please register them with the collision PR instead.
  • Dead code: the periodic and reflective wrap paths (forbidden by the validator), particle_in_domain, s_check_celloutside, s_get_cell, the never-written kahan_comp arrays, and the bubble leftovers that mean nothing for rigid particles (particle_draddt, the radius RK update, Rmax/Rmin_stats_part, the stats file). Commented-out code should be deleted (e.g. kernels 826-877, m_particles_EL 1899-1901).
  • About 330 lines of m_particles_EL are verbatim copies of m_bubbles_EL, and s_mpi_sendrecv_solid_particles duplicates the bubble routine's structure. A shared Lagrangian layer would remove most of this.
  • Gidaspow, Parmar et al. and Osnes et al. are not in docs/references.bib; please cite them with the equation numbers each kernel implements.

What would make this excellent

  1. A conservation regression (one particle in a closed box, total momentum and energy to round-off) and a quiescent-cloud regression. Either would have caught blockers 1-3.
  2. Validation of the curtain example against Wagner et al. (2012): Mach 1.66, a 2 mm curtain at 21 percent volume fraction of 115 um glass, 82.7 kPa ambient is that experiment. Plotting the upstream and downstream curtain fronts against the measured ones would be a strong result, and the example should cite it.
  3. Unit tests of the drag correlations against published tables, including the limits (Stokes, M -> 0, phi -> 0) and continuity at every switch. For the record, Parmar 2010 checks out, including continuity at M = 0.6, 1.0 and 1.75, and the Gidaspow dilute limit matches Schiller-Naumann.
  4. A restart regression: 2N steps must equal N steps, a restart, then N more.
  5. Longer term, one Lagrangian container shared with the bubble model (handover, compaction, restart I/O, RK through rk_coef) that stays on the device.

Comment thread src/simulation/m_particles_EL_kernels.fpp Outdated
Comment thread src/simulation/m_particles_EL_kernels.fpp Outdated
Comment thread src/simulation/m_particles_EL.fpp Outdated
Comment thread src/simulation/m_particles_EL.fpp Outdated
Comment thread src/simulation/m_particles_EL_kernels.fpp Outdated
Comment thread toolchain/mfc/params/definitions.py Outdated
Comment thread toolchain/mfc/case_validator.py Outdated
Comment thread src/post_process/m_data_output.fpp
Comment thread examples/2D_particle_curtain/case.py Outdated
Comment thread examples/2D_particle_curtain/case.py Outdated

@wilfonba wilfonba left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This PR needs to introduce a shared m_euler_lagrange.fpp to house shared utilities for bubbles and particles. As it stands, there are hundreds of lines of almost verbatim duplicate code, and several hundred more of very similar code.

Comment thread examples/2D_particle_curtain/case.py Outdated
Comment thread examples/2D_particle_curtain/case.py
Comment thread examples/2D_particle_hemisphere/case.py Outdated
Comment thread examples/2D_particle_curtain/README.md
Comment thread examples/2D_particle_hemisphere/README.md
Comment thread src/simulation/m_particles_EL.fpp Outdated
Comment thread src/simulation/m_particles_EL.fpp Outdated
Comment thread src/simulation/m_particles_EL.fpp Outdated
Comment thread src/simulation/m_particles_EL.fpp Outdated
Comment thread src/simulation/m_particles_EL.fpp
@thierrydaoud

thierrydaoud commented Sep 29, 2026 •

Copy link
Copy Markdown
Author

Hello all,

I appreciate you taking the time for this pull request, and provide detailed comments and feedback.

I am going over them and will answer/resolve them one by one.

This might take a day or more, but hopefully will be resolved soon.

After resolving most/all comments, I will push the code changes/commits and squash them here, instead of bugging users with notifications about the various commits. I'll be doing them instead on my local branch

Thanks for your patience and assistance.

Lagrangian solid-particle model (particles_lagrange) next to the Lagrangian
bubbles. Rigid spheres are tracked individually; the gas acts on them through
quasi-steady drag (Gidaspow, Parmar et al., Osnes et al.), optional
pressure-gradient and added-mass forces and an optional stochastic drag
fluctuation. With two-way coupling the particles act back on the gas through
momentum and energy sources projected with a Gaussian kernel. No collisions in
phase 1.

- Solver: src/simulation/m_particles_EL.fpp and m_particles_EL_kernels.fpp.
- Shared module m_euler_lagrange.fpp for bubbles and particles: cell location,
  domain and cell-volume helpers, input parsing and start-up, void-fraction and
  evolution files, MPI-IO restart read/write. Bubble results are unchanged.
- Coupling: force deposit normalized by sum(G V); the gas receives the particle
  momentum change m du/dt (including the added-mass reaction) and the work
  -F.u_p; pressure terms -(alpha_p/alpha_f) grad p and
  -(alpha_p/alpha_f) div(p u) as in the bubble solver, so a quiescent cloud
  stays at rest.
- Inputs registered in toolchain/mfc/params/definitions.py, validated by
  check_particles_lagrange (unsupported combinations rejected), documented in
  docs/documentation/case.md with references in docs/references.bib.
- Examples 2D_particle_curtain and 2D_particle_hemisphere with goldens; 12
  particle regression tests, including a quiescent cloud and a closed box.
- Shared-code changes: halo width and buffers, beta exchange, solid-particle
  MPI handover, particle gradients inside the RHS direction loop, parallel-I/O
  volume-fraction slot, post-process naming and rank-count handling.
@thierrydaoud

Copy link
Copy Markdown
Author

Summary

This PR adds a Lagrangian solid-particle model (particles_lagrange) next to the existing Lagrangian bubble model. Rigid spherical particles are tracked individually. The gas acts on them through quasi-steady drag (Gidaspow, Parmar et al., or Osnes et al.), optional pressure-gradient and added-mass forces, and an optional stochastic drag-fluctuation force. With two-way coupling, the particles act back on the gas through momentum and energy source terms, projected onto the grid with a Gaussian kernel.

The solver is ported from our research fork (MFC-EL) and adapted to current master. Phase 1 has no particle–particle collisions. Utilities the bubble and particle solvers had in duplicate now live in a shared module, m_euler_lagrange.

Existing cases are unaffected: every Lagrangian-bubble golden passes unchanged, bit for bit.

Usage and Applications

The model simulates particle-laden compressible flows in the Euler–Lagrange framework, such as shock–particle-curtain interaction and blast waves through particle clouds. Two examples are included and run in CI.

What's included

  • Solver: src/simulation/m_particles_EL.fpp (driver, dynamics, sources, RK update, boundaries, I/O) and m_particles_EL_kernels.fpp (GPU seq kernels: Gaussian projection, interpolation, force model, drag correlations).
  • Shared Euler–Lagrange module: src/simulation/m_euler_lagrange.fpp, used by bubbles and particles: cell location, domain and cell-volume helpers, input-file parsing, start-up (restart point, rank bounds, output directory), the void-fraction and evolution files, and the MPI-IO restart read/write. Each solver keeps only what touches its own state.
  • Inputs: particles_lagrange, particle_pp%rho0ref_particle, particle_params%… and lag_voidfrac_wrt, registered in toolchain/mfc/params/definitions.py.
  • Validation: check_particles_lagrange in case_validator.py, with 19 unit tests. It rejects the combinations phase 1 does not support (see Known limitations).
  • Docs: particle section and parameter table in docs/documentation/case.md; drag-law and model references in docs/references.bib.
  • Examples: examples/2D_particle_curtain (Mach 1.66 shock through a 2 mm glass-particle curtain; the setup of Wagner et al., 2012) and examples/2D_particle_hemisphere (Mach 10 blast through a half-ring of particles). Shock states come from a Mach number via the normal-shock relations; particles come from a separate gen_particles.py that case.py calls only when ./mfc.sh run runs pre_process. READMEs show result images.
  • Regression tests: 12 golden tests: 2D one-way, two-way, two-way on 2 ranks, Gidaspow, Parmar, drag fluctuations, a quiescent cloud and a closed box; 3D two-way on 1 and 2 ranks; and the two examples.

Changes to shared code

  • Shared module m_euler_lagrange (above). The bubble module now uses it; bubble results are unchanged. For bubbles this also adds error checks on every restart-file open, an abort on a rank-count mismatch at restart (it used to read the wrong part of the file), and appending to the evolution file on a restart.
  • Particle field gradients (m_rhs.fpp): computed inside the RHS direction loop from that sweep's reconstructed face states, only when a gradient force is on. No extra copies of the face states are kept.
  • Halo width (m_helper_basic.fpp): s_configure_coordinate_bounds takes particles_lagrange alongside bubbles_lagrange.
  • Smoothed-field (beta) exchange: beta_vars holds up to 11 entries and s_populate_beta_buffers has an optional vars argument; bubble calls are unchanged.
  • Halo buffer (m_mpi_common.fpp): sized for the particle beta exchange, with the bubble case included.
  • MPI particle handover (m_mpi_proxy.fpp): solid-particle send/receive. Allocation and count exchange are shared with bubbles (s_allocate_particle_comm, s_exchange_particle_counts).
  • Parallel I/O slot for the volume-fraction field, and a t = 0 save on fresh particle starts so post-process sees the initial volume fraction.
  • Post-process: one lag_name replaces 14 hard-coded lag_bubbles names, and particles are written for any post-process rank count.
  • Misc: f_xorshift_rand in m_helper.fpp (for the drag fluctuations), _emit_struct_bcast in the Fortran generator, a packer rule for lag_particle output headers, and the Example test harness (particle examples run their own generator; lag_db_wrt is off and t_stop is capped at 5e-5 in that suite).

Physics changes made during review

These change two-way results; all particle goldens were regenerated.

  1. Momentum exchange. The force deposit was normalized by Σ(G V α_f) and then divided by α_f again, so the gas received F/⟨α_f⟩ (1.27 F at the curtain's packing). It is now normalized by Σ(G V).
  2. Added-mass reaction. The particle equation is solved implicitly, (m + m_a) du/dt = f_p, but the gas received −f_p and missed the reaction to the −m_a du/dt part. It now receives −m du/dt.
  3. Energy exchange. The energy source was zero and the gas received S·u_f, so drag dissipation left the system. The gas now receives −F·u_p.
  4. Pressure terms. −(p/α_f)∇α_f and −(p/α_f)u·∇α_f (the expansion of a ∇·(α_f p) form whose balancing interphase term was missing) are replaced by the bubble solver's −(α_p/α_f)∇p and −(α_p/α_f)∇·(p u). A cloud at rest in uniform pressure now stays exactly at rest; before, the same setup reached 22% pressure errors.
  5. Drag floors. The volume-fraction floors and the Gidaspow and Osnes Reynolds floors are removed. The Gidaspow floor cut the Stokes drag by Re/0.1 below Re = 0.1. Parmar keeps its floor, which bounds its log(Re) fits.
  6. Drag-fluctuation model. Advanced once per time step with the exact Ornstein–Uhlenbeck update, a portable seed, and corrected Ma–Ahmadi coefficients.

In a closed box with slip walls (moving particle cloud, gas at rest), total momentum drift went from +14.9% to +1.9% after 100 steps, and from +25.7% to +2.4% after 1000 steps in a longer box, where it no longer grows once the exchange is over. The energy lost early in the exchange went from 78% of the particles' kinetic energy to a few percent. The remainder comes from discretizing the α_f terms and shrinks as the α_f field gets smoother.

Verification

Check Result
Particle goldens (12), CPU, gfortran 15 (macOS) 12/12 pass
Lagrangian-bubble goldens (28), CPU, after the shared-module refactor and the master merge 28/28 pass, bit for bit
Particle (12) and Lagrangian-bubble (28) goldens, GPU, NVHPC 25.9 OpenACC (HiPerGator), at 7b829d67 40/40 pass against the CPU goldens
Particle restart: 20 steps straight vs 10 + restart + 10 flow 4e-16; positions, seeds and drag-fluctuation state identical
Bubble restart (2D two-way, 2 ranks), same check flow 8e-16, bubble state 3e-18
Quiescent cloud (fixed particles, uniform pressure) gas stays exactly at rest

Runs before the review's physics fixes, kept for reference: GPU vs CPU on LLNL Tuolumne (Cray CCE, OpenACC) agreed to 5e-9 in the flow on the hemisphere case; NVHPC 25.9 OpenACC on a HiPerGator GPU node passed the 8 particle tests then present; 1 vs 2 ranks agreed to 1e-16 (2D) and bit for bit (3D) with particles crossing ranks; and a debug build with bounds checking ran clean on multiple ranks.

Bugs found and fixed

During the port: gradient weights computed on the GPU with run-time-sized private arrays (particles about 5% slow on GPU); an unset z-velocity entering the added-mass force in 2D; zero-slip drag NaNs that zeroed the whole particle force; a halo-buffer overflow in the multi-rank beta exchange; unclosed GPU loops and missing private scalars; particle output from stale host copies; and a parallel-I/O crash at the first save.

During review: the four coupling errors above; interpolation overshoot (the barycentric interpolant is now clamped to its stencil range, which removed negative interpolated pressures at blast fronts); a non-finite force now aborts with a report of the particle, its Reynolds and Mach numbers and the force term responsible, instead of being zeroed silently; seed overflow (undefined behaviour in Fortran); restart files that lost the drag-fluctuation state and the seed; and post-process missing particles when run on a different rank count.

Known limitations

The validator rejects the unsupported combinations.

  • Collisions are not included (phase 2).
  • Periodic and reflective boundaries, and axisymmetric coordinates, are rejected.
  • One-way coupling with added mass is rejected: the fluid acceleration it needs is only available with two-way coupling.
  • Restarts need parallel_io = T and the same number of ranks as the run that wrote the file (inherited from the bubble restart format).
  • Added mass across a restart: the fluid acceleration from the previous stage is not saved, so the first stage after a restart uses zero (a difference of about 5e-5 in the tests).
  • Multi-fluid added mass: dρ/dt uses fluid 1's partial density only.
  • Shared container: adding, copying, the RK update, boundary enforcement and the MPI send/receive packing are still per solver. Sharing them needs one Lagrangian state container for both, which is planned with the collision work.
  • Grid-aligned artifact: with HLLC, the hemisphere blast shows a carbuncle along the x = 0 grid line; the example uses HLL.
  • Performance has not been profiled.

Acknowledgement

  • I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

@thierrydaoud
thierrydaoud marked this pull request as ready for review October 2, 2026 03:50
@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.15475% with 573 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.69%. Comparing base (47568a0) to head (257be12).

Files with missing lines Patch % Lines
src/simulation/m_particles_EL.fpp 65.98% 138 Missing and 45 partials ⚠️
src/simulation/m_euler_lagrange.fpp 60.90% 114 Missing and 33 partials ⚠️
src/simulation/m_bubbles_EL.fpp 55.94% 74 Missing and 15 partials ⚠️
src/simulation/m_particles_EL_kernels.fpp 77.84% 30 Missing and 48 partials ⚠️
src/simulation/m_bubbles_EL_kernels.fpp 31.03% 20 Missing ⚠️
src/simulation/m_mpi_proxy.fpp 70.90% 10 Missing and 6 partials ⚠️
src/post_process/m_data_output.fpp 36.36% 13 Missing and 1 partial ⚠️
src/simulation/m_global_parameters.fpp 90.62% 2 Missing and 4 partials ⚠️
src/simulation/m_time_steppers.fpp 20.00% 0 Missing and 4 partials ⚠️
src/pre_process/m_data_output.fpp 0.00% 1 Missing and 2 partials ⚠️
... and 11 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1926      +/-   ##
==========================================
+ Coverage   61.90%   63.69%   +1.79%     
==========================================
  Files          86       89       +3     
  Lines       22885    23736     +851     
  Branches     3369     3433      +64     
==========================================
+ Hits        14166    15118     +952     
+ Misses       6220     6004     -216     
- Partials     2499     2614     +115     

☔ 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
sbryngelson marked this pull request as draft October 2, 2026 16:28

@sbryngelson sbryngelson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few issues from a read of the particle module and kernels. These come from reading the code; I have not built or run them.

Comment thread src/simulation/m_particles_EL.fpp Outdated
Comment thread src/simulation/m_particles_EL_kernels.fpp Outdated
Comment thread src/common/m_helper.fpp Outdated
Comment thread src/simulation/m_particles_EL.fpp
Comment thread src/simulation/m_particles_EL.fpp Outdated
sbryngelson and others added 3 commits October 4, 2026 22:34
…ange, seed round trip

- The particle evolution file resumes its t_save cadence at the restart time
  instead of writing every step until it caught up from t = 0.
- Added mass with several fluids: dρ/dt and the density gradient sum all
  partial densities, matching the mixture density (single fluid unchanged).
- f_xorshift_rand divides by 2^31, so the state -2^31 maps to 1 and the
  uniform stays in [0, 1] (Box-Muller no longer sees log(u) > 0).
- The restart file stores the seed as two 16-bit halves (columns 20-21), exact
  in single and double precision; older files still read correctly.
- The validator requires fluid_pp(l)%cv > 0 when particle_params%suth(l) is
  set (Sutherland's law needs the temperature).

The qs_fluct_force golden is regenerated (RNG rescaling, changes up to 2e-9).
Several test particles started exactly on cell faces of the test grids, and
x = 0.5 is also the 2-rank boundary, so round-off decided their cell and rank.
That made the goldens compiler- and precision-dependent: qs_fluct_force failed
on every NVHPC CPU lane, and the 2-rank test failed in single precision, where
particles at x = 0.5 stayed on the other rank. The particles now start off
every face and boundary; the moving ones still cross ranks, now at definite
times (in 3D they also move across the z = 0.5 split). Particle goldens are
regenerated; a debug build and a single-precision build now match them.
@thierrydaoud
thierrydaoud marked this pull request as ready for review October 8, 2026 00:53
@sbryngelson
sbryngelson marked this pull request as draft October 8, 2026 16:50
# Conflicts:
#	src/common/m_helper.fpp
On NVHPC CPU builds the two-pass inliner (-Mextract/-Minline) inlined the
RNG into s_compute_qs_fluctuations and returned the last of the five draws
for all five, so Z1 = Z2 and the drag and lift fluctuations were neither
independent nor Gaussian. This is why qs_fluct_force failed on every NVHPC
CPU lane. gfortran, Intel, Cray and NVHPC GPU builds drew correct numbers,
so the goldens are unchanged.

- s_xorshift_rand is excluded from -Minline, as other miscompiled routines are.
- The RNG is a subroutine, s_xorshift_rand(var, seed), like s_prng, instead
  of a function that updates its argument.

Verified on NVHPC 25.9 CPU (HiPerGator): the five draws match gfortran and
EDD3540A passes; all 10 particle tests pass with gfortran.

Committed with --no-verify: precheck steps run by hand on macOS (all pass
except the SLURM-monitor/preflight pytest files that fail on clean master here).
@thierrydaoud
thierrydaoud marked this pull request as ready for review October 8, 2026 20:26
@thierrydaoud

Copy link
Copy Markdown
Author

NVHPC CPU lanes: root cause and fix (f82c096)

The NVHPC CPU lanes (23.11–26.3) had failed on every commit of this PR, always on EDD3540A (2D, two-way coupling, qs_fluct_force). It wasn't a tolerance or golden issue. NVHPC CPU builds were generating wrong random numbers for the drag fluctuations.

Cause. The two-pass cross-file inliner (-Mextract / -Minline) inlines the xorshift RNG from m_helper into s_compute_qs_fluctuations. The routine draws five uniforms per call. After inlining, nvfortran performs all five state updates but returns the last draw for every entry:

gfortran:        0.4151  0.4525  0.1495  0.0052  0.8901
NVHPC 25.9 CPU:  0.8901  0.8901  0.8901  0.8901  0.8901

So Z1 = Z2, and the Box-Muller output depended on a single uniform. It wasn't Gaussian (mean ≈ 0.06, variance ≈ 1.12) or independent between the drag and lift components. Only NVHPC CPU release builds were affected; gfortran, Intel, Cray and NVHPC GPU drew correct numbers. I found it by printing every intermediate of the fluctuation routine from gfortran and NVHPC 25.9 CPU builds; the random draws were the first values that differed. A standalone single-file test with the same RNG was correct, because the cross-file inliner wasn't involved. This is the same class of miscompile as the m_pressure_relaxation case already documented in cmake/MFCTargets.cmake.

Fix.

except:s_xorshift_rand is added to the existing -Minline exception list, so the RNG stays a real call.
The RNG is now a subroutine, s_xorshift_rand(var, seed), matching s_prng, instead of a function that modifies its argument. This alone didn't fix the NVHPC output, but it removes the side-effecting function.
Goldens are unchanged.

Verified.

gfortran: all 10 Lagrange Particles tests pass.
HiPerGator, NVHPC 25.9: CPU (--no-gpu) 10/10 (EDD3540A now passes) and OpenACC GPU 10/10, both on the current goldens.
./mfc.sh precheck passes.

@thierrydaoud
thierrydaoud marked this pull request as draft October 8, 2026 20:54
The xorshift seeds (particle ID + 16 xorshift rounds) gave correlated
streams for neighboring IDs: the first draws of consecutive particles had a
correlation of +0.35, because xorshift is linear and warm-up rounds keep it
so. The fluctuation model now uses the existing s_prng_splitmix32, whose
hashed Weyl sequence gives unrelated streams per seed (correlation -0.009),
and s_xorshift_rand is removed.

- Each particle's seed is its global ID; particle_seed is the 64-bit
  splitmix state, sent between ranks as MPI_INTEGER8 (buffer size updated)
  and still saved as two 16-bit halves in restart columns 20-21.
- s_prng_splitmix32 and f_mulmod32 are GPU seq routines; the -Minline
  exception moves to s_prng_splitmix32.
- EDD3540A golden regenerated (new random stream); the other particle
  goldens are unchanged.

Checked with gfortran: 10/10 particle tests; 20 steps vs 10 + restart + 10
give bit-identical checkpoints (flow and particles, with seeds); 1 vs 2 ranks
with particles crossing the boundary: identical seeds, states within 1e-16.

Committed with --no-verify: precheck steps run by hand on macOS.
…ameter array

CCE OpenACC rejects the duidxj_id(3, 3) parameter array in s_get_particle_force
("Global in accelerator routine without declare"); NVHPC places it on the
device implicitly. du_i/dx_j is field dufxdx_id + 3*(i - 1) + j - 1, so both
uses compute it like the neighboring dPx_id + dir - 1. Results unchanged.

Committed with --no-verify: format and lint run by hand on macOS; 10/10
particle tests pass with gfortran against the existing goldens.
@thierrydaoud
thierrydaoud marked this pull request as ready for review October 9, 2026 14:10
With CCE OpenMP offload, simulation aborted at startup with "Present Table
Collision": m_bubbles_EL and m_particles_EL each declare a device array named
kahan_comp, and the runtime registered the second at the first one's address.
Every test on that build failed, not only particle tests. A module-specific
name, as m_muscl uses for is1_muscl, avoids the clash. Results unchanged.

Committed with --no-verify: format and lint run by hand on macOS; 10/10
particle tests pass with gfortran against the existing goldens.
@wilfonba
wilfonba self-requested a review October 10, 2026 14:47
@wilfonba
wilfonba dismissed their stale review October 10, 2026 14:47

Requested fresh review

p_owner_rank was written at init, every stage and on receive but never
read; lag_recv_ranks only fed it. With it gone the second n_recv count in
s_begin_lag_entity_exchange duplicated the first. (The deallocation part of
the review comment was already fixed by the allocation change.)

Lagrangian tests pass (47/47 incl. five 2-rank cases, macOS CPU+MPI).
Committed with --no-verify: precheck hangs on macOS.
- coupling = two_way requires pressure_gradient_force = T: the gas
  momentum source carries the full -grad p, so without the force the gas
  also takes the particles' share.
- Reject body forces (particles get no gravity; added mass would read g
  as a fluid acceleration), bubbles_euler, ib, a state-dependent EOS, and
  the bubble-only lag_*_wrt outputs, which wrote other quantities for
  particles.
- Require particle_pp%rho0ref_particle > 0.
- Default particle input_path is input/particles.dat, the name every
  example, test and doc uses.

Validator tests pass; all examples validate; Lagrangian tests pass
(47/47, macOS CPU+MPI). Committed with --no-verify: precheck hangs on macOS.
Unknown names stopped at the JSON schema check with a bare "data must not
contain" message, so the "did you mean" suggestions never showed. Check
them first. A renamed parameter names its replacement; the first entry is
lag_params%solver_approach -> lag_params%coupling.

Toolchain tests pass. Committed with --no-verify: precheck hangs on macOS.
The particle kernel width is fixed in grid cells by kernel_alpha; the
parameter came over with the bubble set (where it scales the kernel
standard deviation) and was never read. lag_params%epsilonb is kept.

Toolchain tests pass; all examples validate; Lagrangian tests pass
(47/47, macOS CPU+MPI). Committed with --no-verify: precheck hangs on macOS.
The particle kernel spans mapCells_loc cells past the host cell and its
normalization includes ghost cells, but only periodic and reflective
boundaries folded the ghost share back, so at slip and no-slip walls it
was dropped. Measured on a 2D slip-wall box (kernel_alpha = ln 10), the
interior kept 0.91667 of alpha_p, alpha_p*u_p, Sm and SE for a particle
centered in a wall-adjacent cell and 0.72028 for one at y = r_p. Wall
boundaries now use the reflective fold (ghost share added to the mirror
cell, then ghost = mirror) when particles_lagrange is on; the interior
then keeps 1.00000 in both cases and in a corner. Bubble runs are
unchanged.

No golden changes: no test particle's kernel reaches a wall ghost cell.
Lagrangian tests pass (47/47, macOS CPU+MPI). Committed with --no-verify:
precheck hangs on macOS.
The beta halo was sized for num_beta_vars_max = 11 fields on every
Euler-Lagrange run, including bubble runs that exchange 2-3 and pre/post
processing, which exchange none. s_initialize_mpi_common_module now takes
num_beta_vars_in (simulation: the larger of bub_q_idx%num and
part_q_idx%num for the active models; pre/post: 0). Abort if it exceeds
num_beta_vars_max, or if s_populate_beta_buffers is asked for more fields
than the halo holds; 3D two-way particles already use all 11.

Lagrangian tests pass (47/47 incl. 2-rank, macOS CPU+MPI). Committed with
--no-verify: precheck hangs on macOS.
Committed with --no-verify: precheck hangs on macOS.
2D -> Lagrange Particles -> Two-way Coupling -> Closed Box -> Wall Layer:
six particles in two clusters one radius off the y = 0 and x = 0 slip
walls, moving into them, with valmaxvoid = 0.1 (the clusters' cells reach
alpha_p = 0.124). It covers the wall ghost-cell fold, the radius wall
clamp and sources in cells at the void limit, which no test reached.
Reverting each fix alone fails it: 4288288 (rel 9.1e-6), 8e69a80
(rel 4.3e-3), de6b02d (rel 3.4e-4).

Golden generated on macOS, GNU 16.2, MPI. Lagrangian tests pass (48/48).
Committed with --no-verify: precheck hangs on macOS.
@thierrydaoud

thierrydaoud commented Oct 24, 2026 •

Copy link
Copy Markdown
Author

@sbryngelson @wilfonba

I am adding particle restart tests for the lagrange particles. Although the lagrange bubbles do have restart tests, they are not triggered with a full test. The full test requires parallel_io =F meanwhile the restart tests for the lagrangian bubbles and particles require parallel_io = T.

I will make that fix for particles_lagrange, to allow serial restart too parallel_io =F, and added a validator rule for bubbles_lagrange that "restarts require parallel_io = T" so that a serial bubble restart fails at validation.

I think the bubbles also previously failed at runtime if the serial output was enabled. that is also now checked in the validator.

thierrydaoud and others added 12 commits October 24, 2026 16:07
The Lagrangian restart file is its own MPI-IO file, but it was only
written and read with parallel_io because mpiiofs and mpi_info_int were
set up only then (the gate came with the bubble writer). Set mpiiofs and
mpi_info_int = MPI_INFO_NULL in both modes (hints still only with
parallel_io), drop the gate in the shared reader and the particle writer,
and create restart_data when missing. The file name is the same in both
modes.

Validator: particle restarts no longer need parallel_io = T; bubble
restarts now do (their writer keeps the gate), which was only caught at
run time before.

A 2D two-way qs_fluct_force run restarted at step 25 with serial I/O
matches the straight run bitwise (cons, beta, particle evolution).
Toolchain tests pass; all examples validate; 113 tests pass (all
Lagrangian and restart cases plus 60 random, macOS CPU+MPI).
Committed with --no-verify: precheck hangs on macOS.
The --test-all template set fd_order = 1 for 3D vorticity output, which
the particle validator rejects, so post_process never ran for 3D particle
tests (test.py does not check the post_process return code). Particles
use fd_order 2 or 4, which also suits the vorticity output.

3C74B315 now runs pre_process, simulation and post_process and writes its
Silo files. Committed with --no-verify: precheck hangs on macOS.
2D -> Lagrange Particles -> Two-way Coupling -> qs_fluct_force -> Restart
runs to step 25 with serial I/O, restarts from the particle restart file
and must match the straight run, so the saved seed and fluctuation state
are covered. Added mass is off: its fluid acceleration is not saved.
Skipped without MPI, since the restart file is MPI-IO. Replacing the seed
restore with a constant fails it (cons.4 mismatch at step 50).

Golden generated on macOS, GNU 16.2, MPI. Committed with --no-verify:
precheck hangs on macOS.
Both examples have particles on the y = 0 slip wall (the hemisphere's
symmetry plane, the curtain's particles at y = r_p), whose wall ghost-cell
deposit 8e69a80 now folds back instead of dropping. Step-0 alpha_f moves
by up to 1.3e-3 (hemisphere) and 2.2e-3 (curtain) relative near the wall;
after one step cons.4 differs by up to 2.3e-2. Reverting 8e69a80 alone
restores the old goldens. Regenerated on macOS, GNU 16.2, MPI.

Lagrangian tests incl. examples pass (52/52). Committed with --no-verify:
precheck hangs on macOS.
fd_order = 4 sets the widest particle halo; interpolation_order = 6 is the
widest stencil the validator allows with the gradient forces at
fd_order = 2. Both run in 2D and 3D on 2 ranks, so the stencils cross a
rank boundary. Each matches its 1-rank run to round-off (max difference
/ field max 5e-13 to 2e-12, as for the existing fd_order = 2 2-rank
cases), and each differs from the order-2 runs (fields 2e-3 to 4e-2).

Goldens generated on macOS, GNU 16.2, MPI. Lagrangian tests incl.
examples pass (56/56). Committed with --no-verify: precheck hangs on macOS.
- Remove dead code: use m_ibm and the adap_dt_tol default in
  m_particles_EL, the particle cyl_coord branch (the validator forbids
  it), the unused locals of s_write_lag_bubble_evol, FMT in
  s_open_lag_bubble_stats (the header is written with *), and the empty
  pre_process comment.
- s_prng_splitmix32 and f_mulmod32 use seed_kind instead of local
  int64_kind parameters.
- Size the beta halo and the Lagrangian MPI buffers through
  integer(kind=8) variables instead of _8 literals and int(..., 8).
- s_read_lag_input opens with newunit and status='old', and aborts if the
  file cannot be opened (it read zero entities before).
- s_sync_bubbles_for_save keeps the bubble host updates and radius check
  in m_bubbles_EL, as s_sync_particles_for_save does for particles; the
  second host update after the data write was redundant.
- Renumber the force-kernel step comments.

Lagrangian, restart and 2-rank tests pass (66/66, macOS CPU+MPI).
Committed with --no-verify: precheck hangs on macOS.
From the review: QS_Parmar/QS_Osnes/QS_Gidaspow -> f_qs_parmar/
f_qs_osnes/f_qs_gidaspow; the viscosity argument of s_get_particle_force
Re -> mu_fluid (and myRe -> my_mu in its caller); s_add_particles ->
s_add_particle; s_compute_particle_EL_dynamics ->
s_compute_particles_EL_dynamics, to match s_compute_particles_EL_source.
Also s_finalize_particle_lagrangian_solver -> s_finalize_particles_EL_module,
to mirror s_initialize_particles_EL_module.

examples/3D_moving_lag_particles is a bubble case: renamed to
examples/3D_moving_lag_bubbles (it is on the example-test skip list, so no
golden moves).

Lagrangian tests incl. examples pass (56/56); the example validates.
Committed with --no-verify: precheck hangs on macOS.
…_bcast

The four per-struct emitters were variants of _emit_struct_bcast with the
MPI types hard-coded (bub_pp all REAL, chem_params LOG/INT only, rburn and
lag_params split by type). _emit_struct_bcast takes each member's type
from the registry, and _mpi_type_for raises on an unmapped type, so a new
member can no longer be silently skipped.

Generated broadcasts: pre and post identical; sim has the same 313 calls,
reordered alphabetically within lag_params and chem_params. Toolchain
tests pass; 85 Lagrangian, chemistry, reactive-burn and multi-rank tests
pass. Committed with --no-verify: precheck hangs on macOS.
b55ff34 only carried the example directory move; this adds the rest of
that change: QS_Parmar/QS_Osnes/QS_Gidaspow -> f_qs_parmar/f_qs_osnes/
f_qs_gidaspow, the Re argument of s_get_particle_force -> mu_fluid (myRe ->
my_mu in its caller), s_add_particles -> s_add_particle,
s_compute_particle_EL_dynamics -> s_compute_particles_EL_dynamics,
s_finalize_particle_lagrangian_solver -> s_finalize_particles_EL_module,
and the example-test skip list entry for examples/3D_moving_lag_bubbles.

Lagrangian tests incl. examples pass (56/56). Committed with --no-verify:
precheck hangs on macOS.
The three branches differed only in the moving index and the weights
array, and narrowed their loops by a hard-coded 2. One #:for body now
generates the three GPU loops, with the narrowing taken from npts
(fd_order/2) along dir only.

With fd_order = 2 the loops now also fill the outermost ghost layer,
where weights exist; nothing reads it: 2D and 3D two-way runs are
bitwise identical to before. Lagrangian tests incl. examples pass
(56/56). Committed with --no-verify: precheck hangs on macOS.
s_beta_periodic, s_beta_extrapolation and s_beta_reflective declared
q_beta and kahan_comp as dimension(num_dims + 1), the bubble field count.
Since 8e69a80 slip walls call s_beta_reflective for particles, whose
fields go up to index 8 in 2D and 11 in 3D: the reldebug CI job aborted on
both particle examples ("Index '6' of dimension 1 of array 'q_beta' above
upper bound of 3"). Release builds touched the right memory, so results
were correct, but the access was out of the declared bounds. Two-way
bubbles with cluster_type >= 4 (up to 5 fields) exceeded it too.
dimension(*) keeps the explicit-shape calling convention (no descriptor
in the device routines) and fits any field count.

Reproduced locally in reldebug, then fixed: particle examples and all 56
Lagrangian tests pass in reldebug and release. Committed with
--no-verify: precheck hangs on macOS.
@github-actions

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/simulation/m_particles_EL.fpp 919 +919
src/simulation/m_bubbles_EL.fpp 1020 -619
src/simulation/m_euler_lagrange.fpp 605 +605
src/simulation/m_particles_EL_kernels.fpp 581 +581
src/simulation/m_mpi_proxy.fpp 447 -87
src/simulation/m_bubbles_EL_kernels.fpp 439 -77
src/simulation/m_global_parameters.fpp 864 +69
src/common/m_derived_types.fpp 529 +46
src/common/m_mpi_common.fpp 1512 +18
src/simulation/m_rhs.fpp 1983 +12
src/common/m_helper.fpp 534 +11
src/common/m_constants.fpp 98 +9
src/common/m_global_parameters_common.fpp 257 +7
src/common/m_boundary_common.fpp 454 +6
src/common/m_helper_basic.fpp 123 +5
src/post_process/m_data_output.fpp 1262 +5
src/simulation/p_main.fpp 78 +5
src/post_process/m_global_parameters.fpp 415 +1
src/post_process/m_start_up.fpp 793 +1
src/pre_process/m_start_up.fpp 505 +1
src/simulation/m_start_up.fpp 1272 +1
src/simulation/m_time_steppers.fpp 909 -1
Directory Lines Diff
common 10638 +102
pre_process 5035 +1
simulation 29977 +1408
post_process 3506 +7
total 49156 +1518

end subroutine s_set_lag_comm_coords

!> Create case_dir/D on rank 0 if it does not exist.
impure subroutine s_create_D_dir()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i'm pretty sure this function already exists somewhere else, also i dont see how it's EL specific

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

can be fixed


!> Read the initial Lagrangian entities from path, one per line with ncols values, and return those in this rank's physical
!! domain: rows(1:n_loc, :) and their global IDs ids(1:n_loc), which are the line numbers among the n_read lines read.
impure subroutine s_read_lag_input(path, ncols, n_glb, rows, ids, n_loc, n_read)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should this not go in m_data_input / m_startup?

end subroutine s_read_lag_input

!> Sum the local count n_loc over the ranks into n_glb and abort if no entity lies in the domain.
impure subroutine s_count_lag_entities_glb(n_loc, n_glb, path)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

usually global routines go elsewhere (like module startup calls)


!> Open this rank's evolution file D/lag_<lag_kind>_evol_<rank>.dat: on a fresh start replace it and write the column headers
!! (first header width 14, the others width), on a restart append to it.
impure subroutine s_open_lag_evol(lag_kind, width, headers)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you seem to be mixing i/o with start up with computation

call MPI_TYPE_FREE(view, ierr)
#endif

end subroutine s_write_lag_restart

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

isn't this what we have m-data-output for?


end subroutine s_write_lag_restart

!> Read restart_data/lag_<lag_kind>_<save_count>.dat written by s_write_lag_restart: set mytime and dt from the header and

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

more stuff cluttering the code/algorithm?

@thierrydaoud

Copy link
Copy Markdown
Author

@sbryngelson

I went over the most recent comments. I just want to be clear first that these exist for the lagrangian bubbles, and were copied over to be shared with the lagrangian solid particles in this shared module.

The one i commented on "can be fixed" is something we can instead fix by creating D directory once at simulation start-up, and drop both the helper and its two call sites.

The rest of the comments can be worked on in a follow-up PR, in which the input and restart headers are moved to start-up, since that will need restructuring the bubble init.

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.

4 participants