Moving EL Bubbles with MPI Decomposition#1290
Conversation
sbryngelson
force-pushed
the
MovingBubblesFresh-clean
branch
2 times, most recently
from
a4e5fb0 to
c0f6323
Compare
Claude Code ReviewIncremental review from: Two files changed since the last review. No new high-confidence issues.
|
|
<!-- claude-review: thread=primary; reviewed_sha=caf85b0db59c419886f36ae8a0bbf3c07ccbfe9c; mode=incremental --> |
sbryngelson
force-pushed
the
MovingBubblesFresh-clean
branch
from
196d171 to
d030b8a
Compare
sbryngelson
force-pushed
the
MovingBubblesFresh-clean
branch
from
a363081 to
52e8af1
Compare
sbryngelson
force-pushed
the
MovingBubblesFresh-clean
branch
9 times, most recently
from
fcb2c84 to
aad0637
Compare
Merged current
|
…tate; adopt master's IB-patch parallelism refactor (MFlowCode#1603/MFlowCode#1549) post_process m_global_parameters: kept MFlowCode#1290 nidx/neighbor_ranks + master's ib_airfoil decls. m_ib_patches: took master's two-mode s_apply_ib_patches dispatcher and re-applied MFlowCode#1290's x_domain->glb_bounds substitution (10 sites, both parallelism modes) so IB periodic wrapping uses the global extent under MPI decomposition. Verified: 3-target CPU build, precheck clean.
…lfong into MovingBubblesFresh-clean
…ed send-guard condition
|
@wilfonba - a few fixes pushed to this branch, plus one left for you. Pushed (f4acac0):
Left for you - precision bug in the Kahan compensation: (The |
The relevant computations for the capillary CFL condition were added back by a850436 |
|
Leaving this coment here for myself to edit and comment on each item as I address it.
This is fine, besides this restructuring being weird.
These belong in m_boundary_common, so this is all good.
This looks fine
These all look fine
This is fixed. Kahan compensation is now an optional feature that defaults to true. If it's true, and simulation is run with |
…_advection example
…esting infrastructure to better understand what leads to two-way coupled EL bubble simulations diverging when ran with MPI
# Conflicts: # src/common/m_helper.fpp # toolchain/mfc/params/definitions.py
There was a problem hiding this comment.
Pull request overview
This PR extends MFC’s Euler–Lagrange (EL) bubble capability to support moving bubbles under MPI decomposition, adds new EL translational/force-model parameters, and updates I/O + validation + toolchain infrastructure to support the new workflows (including restart/parallel-IO compatibility and new example/test matrices).
Changes:
- Add new Lagrangian-bubble parameters (e.g.,
vel_model,drag_model,pressure_force,gravity_force,write_void_evol,input_path,charNz) and enforce constraints in the case validator + docs. - Rework several simulation/pre/post-process paths around global-domain bounds (
glb_bounds) and bubble-related halo/buffer handling. - Add/adjust tests and examples, including a dedicated MPI-rank discrepancy isolation campaign for EL coupling.
Reviewed changes
Copilot reviewed 95 out of 128 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| toolchain/templates/phoenix_ifx.mako | New Phoenix IFX SLURM/run-script template. |
| toolchain/modules | Add Phoenix IFX module group; minor whitespace cleanup. |
| toolchain/mfc/test/test.py | Adjust nvfortran-in-Docker skip logic. |
| toolchain/mfc/test/cases.py | Extend Lagrange bubble test matrix; update example skip lists. |
| toolchain/mfc/test/case.py | Ensure generated lag_bubbles.dat ends with newline. |
| toolchain/mfc/params/generators/fortran_gen.py | Broadcast lag_params members incl. STR; fail fast on unhandled ParamTypes. |
| toolchain/mfc/params/descriptions.py | Add parameter descriptions/patterns for new bubble + interface inputs. |
| toolchain/mfc/params/definitions.py | Extend bubble_model choices; register new inputs (interface + lag params) and adjust _nv target sets. |
| toolchain/mfc/case_validator.py | Add validation for new lagrange bubble parameters; document Kahan/mixed constraint. |
| toolchain/bootstrap/precheck.sh | Export temp directory var for parallel precheck phases. |
| tests/F628CCC9/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/E4FA12EC/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/D64170CF/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/BAC48125/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/A9085DAF/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/A04A396A/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/7854BA64/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/630E2099/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/549393AA/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/54632DFD/golden-metadata.txt | Add new golden metadata for regression test. |
| tests/404D1458/golden-metadata.txt | Add new golden metadata for regression test. |
| src/simulation/m_time_steppers.fpp | Update EL integration calls; change CFL dt computation to a scalar-reduction approach; bubble evolution write gating. |
| src/simulation/m_start_up.fpp | Track periodic BC flags; use global-grid distribution helper; update EL init APIs; update NaN checks to local EL bubble count. |
| src/simulation/m_sim_helpers.fpp | Refactor CFL/stability computations; add capillary CFL handling; change APIs to scalar outputs. |
| src/simulation/m_rhs.fpp | Restructure EL dynamics/source evaluation by solver approach. |
| src/simulation/m_ibm.fpp | Replace local x_domain/y_domain/z_domain with glb_bounds and update device updates. |
| src/simulation/m_ib_patches.fpp | Use glb_bounds for periodic patch centers. |
| src/simulation/m_global_parameters.fpp | Introduce glb_bounds, periodic_bc, bubble/MPI neighbor state, and new lagrange bubble globals. |
| src/simulation/m_compute_levelset.fpp | Use glb_bounds for periodic IB geometry calculations; update doc comment. |
| src/simulation/m_collisions.fpp | Use glb_bounds for periodic IB collisions and ownership logic. |
| src/simulation/m_bubbles_EE.fpp | Adjust adaptive-time-step convergence reporting under GPU parallelism. |
| src/pre_process/m_start_up.fpp | Initialize combined BC structure (bc). |
| src/pre_process/m_mpi_proxy.fpp | Broadcast additional hardcoded interfacial-IC variables. |
| src/pre_process/m_global_parameters.fpp | Add pre_process-local lag_params/FD tracking; add neighbor-rank state and hardcoded interface inputs. |
| src/pre_process/m_data_output.fpp | Write an extra bubbles_lagrange field to restart outputs (serial + MPI-IO paths). |
| src/post_process/m_start_up.fpp | Initialize combined BC structure (bc). |
| src/post_process/m_global_parameters.fpp | Add bc and decomposition neighbor state; deallocate neighbor ranks in finalize. |
| src/post_process/m_data_input.f90 | Require q_cons_vf(beta_idx) file presence; use global-grid distribution helper for restart grids. |
| src/common/m_helper.fpp | Docstring cleanups. |
| src/common/m_helper_basic.fpp | Extend coordinate-bound setup to account for bubble FD halos; docstring cleanups. |
| src/common/m_finite_differences.fpp | Docstring update for buffer-size parameter. |
| src/common/m_derived_types.fpp | Extend bubbles_lagrange_parameters with new motion/force/I/O controls. |
| src/common/m_compile_specific.f90 | Make Intel inquire check succeed for either file or directory. |
| src/common/m_boundary_common.fpp | Add beta-buffer bounds and beta-buffer population routines; compute glb_bounds via reductions. |
| src/common/include/SharedHardcoded.fpp | New shared hardcoded deallocation Fypp macro. |
| src/common/include/macros.fpp | Comment cleanup in GPU unified-memory hint macro header. |
| src/common/include/2dHardcodedIC.fpp | Remove redundant comment line. |
| examples/3D_lagrange_shbubcollapse/case.py | Enable write_void_evol. |
| examples/3D_lagrange_coupling_matrix/run_phase3_boundary.sh | New scripted campaign phase for boundary/migration diagnostics. |
| examples/3D_lagrange_coupling_matrix/run_phase2_chaos.sh | New scripted campaign phase for chaos vs rank-divergence baseline. |
| examples/3D_lagrange_coupling_matrix/run_phase1_matrix.sh | New scripted feature-bisection matrix (M0–M3 × ranks). |
| examples/3D_lagrange_coupling_matrix/run_phase0_determinism.sh | New scripted determinism audit phase. |
| examples/3D_lagrange_coupling_matrix/run_case.sh | New helper to run/snapshot a single matrix cell. |
| examples/3D_lagrange_coupling_matrix/run_all.sh | New driver to build and run all phases + Kahan probe. |
| examples/3D_lagrange_coupling_matrix/README.md | New documentation for the isolation campaign. |
| examples/3D_lagrange_coupling_matrix/FINDINGS.md | New report of campaign results/interpretation. |
| examples/3D_lagrange_coupling_matrix/compare_particles.py | New tool to diff particle trajectories across runs. |
| examples/3D_lagrange_coupling_matrix/compare_fields.py | New tool to diff restart-field snapshots across runs. |
| examples/3D_lagrange_bubblescreen/input/README.txt | Remove outdated README (bubble IC now generated). |
| examples/3D_lagrange_bubblescreen/case.py | Generate bubble cloud input file programmatically; adjust save cadence; JSON indenting. |
| examples/2D_lagrange_bubblescreen/input/README.txt | Remove outdated README (bubble IC now generated). |
| examples/2D_lagrange_bubblescreen/case.py | Generate bubble cloud input file programmatically; set 2D char depth; adjust save cadence; JSON indenting. |
| docs/documentation/case.md | Document new interfacial-flow inputs + expanded Lagrange bubble parameter set. |
| .gitignore | Adjust ignore patterns (notably add CSV ignore under examples). |
| if (bubbles_lagrange) then | ||
| block | ||
| real(stp), allocatable :: beta_ones(:,:,:) | ||
| character(LEN=len_trim(t_step_dir) + name_len) :: beta_file_loc | ||
| integer :: jj, kk, ll | ||
| allocate (beta_ones(0:m,0:n,0:p)) | ||
| do ll = 0, p | ||
| do kk = 0, n | ||
| do jj = 0, m | ||
| beta_ones(jj, kk, ll) = 1.0_stp | ||
| end do | ||
| end do | ||
| end do | ||
| write (beta_file_loc, '(A,I0,A)') trim(t_step_dir) // '/q_cons_vf', sys_size + 1, '.dat' | ||
| open (1, FILE=trim(beta_file_loc), form='unformatted', STATUS=status) | ||
| write (1) beta_ones | ||
| close (1) | ||
| deallocate (beta_ones) | ||
| end block | ||
| end if |
| if (bubbles_lagrange) then | ||
| block | ||
| real(stp), allocatable :: beta_ones(:,:,:) | ||
| integer :: jj, kk, ll | ||
| allocate (beta_ones(0:m,0:n,0:p)) | ||
| do ll = 0, p | ||
| do kk = 0, n | ||
| do jj = 0, m | ||
| beta_ones(jj, kk, ll) = 1.0_stp | ||
| end do | ||
| end do | ||
| end do | ||
| call MPI_FILE_WRITE_ALL(ifile, beta_ones, data_size*mpi_io_type, mpi_io_p, status, ierr) | ||
| deallocate (beta_ones) | ||
| end block | ||
| end if |
| if (bubbles_lagrange) then | ||
| block | ||
| real(stp), allocatable :: beta_ones(:,:,:) | ||
| integer :: jj, kk, ll | ||
| allocate (beta_ones(0:m,0:n,0:p)) | ||
| do ll = 0, p | ||
| do kk = 0, n | ||
| do jj = 0, m | ||
| beta_ones(jj, kk, ll) = 1.0_stp | ||
| end do | ||
| end do | ||
| end do | ||
| var_MOK = int(sys_size + 1, MPI_OFFSET_KIND) | ||
| disp = m_MOK*max(MOK, n_MOK)*max(MOK, p_MOK)*WP_MOK*(var_MOK - 1) | ||
| call MPI_FILE_SET_VIEW(ifile, disp, mpi_io_p, MPI_IO_DATA%view(1), 'native', mpi_info_int, ierr) | ||
| call MPI_FILE_WRITE_ALL(ifile, beta_ones, data_size*mpi_io_type, mpi_io_p, status, ierr) | ||
| deallocate (beta_ones) | ||
| end block | ||
| end if |
| if (n == 0) return | ||
| @:DEALLOCATE(y_cb, y_cc, dy) | ||
|
|
||
| if (p == 0) return | ||
| @:DEALLOCATE(z_cb, z_cc, dz) | ||
|
|
||
| if (allocated(neighbor_ranks)) then | ||
| @:DEALLOCATE(neighbor_ranks) | ||
| end if |
| # Skip tests that fail under nvfortran in Docker (pass natively/Apptainer): | ||
| # - Lagrange Bubbles: NaN on 24.1/24.3 (other versions not verified in Docker) | ||
| # - 3D_rayleigh_taylor_muscl: segfaults with nvfortran+MPI (seccomp/mprotect) | ||
| if os.environ.get("FC") == "nvfortran" and os.path.exists("/.dockerenv"): | ||
| nvhpc_skip_uuids = {"B9553426", "4A1BD9B8", "0D1FA5C5", "2122A4F6"} | ||
| nvhpc_skip_uuids = {} | ||
| nvhpc_skip_traces = {"rayleigh_taylor_muscl"} |
|
|
||
| $:GPU_PARALLEL_LOOP(collapse=3, private='[vel, alpha, Re, rho, vel_sum, pres, gamma, pi_inf, c, H, qv, fl]') | ||
| dt_local = huge(1.0_wp) | ||
| $:GPU_PARALLEL_LOOP(collapse=3, private='[vel, alpha, Re, rho, vel_sum, pres, gamma, pi_inf, c, H, qv, fl]', & |
There was a problem hiding this comment.
Need to add variable max_dt in private clause, this is why a few openmp cases with cfl_adap_dt = T are failing on frontier, everything else seems to pass
Description
This PR adds support for moving Euler-Lagrange bubbles and various force models. Tracer particles that simply follow the background flow are implemented, as are Newton's 2nd Law particles that respond to body forces, pressure gradients, and drag. This PR is marked as a draft for now as there are several things that I need to complete before a proper merge can occur, but I wanted these changes to be visible to other contributors working on related features.
Type of change
Testing
16 rank CPU versus 16 rank GPU
This test was ran using the
examples/3D_lagrange_bubblescreentest case and compares the results for a 16 rank CPU simulation to a 16 rank GPU simulation. The visualization shows the following things:test.mp4
2D Lagrange bubbles in crossflow
The following tests all use the
examples/2D_lagrange_in_crossflowexample case. The first crossflow velocity is 0.2. This means that regardless of the drag model, the EL bubbles should accelerate to a velocity of 0.2 and then maintain a constant velocity, with the acceleration dictated by the drag model. The drag models in increasing order of drag force are slip drag -> no-slip drag -> Levich drag, and the results below agree with that.The following three images show comparisons between 1 GPU, 4 GPU, and 4 CPU simulations. The left-hand axis shows the quantity of interest, and the right-hand axis shows the error on a log scale. The errors are nearly identical to zero in all cases, so the error lines are rarely visible. The left column shows position, and the right column shows velocity. The five rows correspond to the five different bubbles in the initial condition.
Slip Drag
I also performed a simulation with "tracer particles" that match the carrier velocity instantly. The results below show the expected behavior.
3D Lagrange Bubblescreen
I also ran the
examples/3D_lagrange_bubblescreenexample case with different numbers of CPUs and GPUs to ensure the results match. The figure below shows that the maximum pressure difference at the center of the bubble screen is on the order of10^-15.2D Lagrange rising bubble
The following test uses the
examples/2D_lagrange_rising_bubbleexample case. Five bubbles start at rest in a domain subject to gravitational force in hydrostatic equilibrium. The bubbles begin moving upwards, but reach a terminal velocity due to drag. The following figure shows the results for 1 GPU, 4 GPUs, and 4 CPUs. The left-hand axis shows the bubble position, and the right-hand axis shows the difference between results. Errors do grow as large as 10^-13, though there does not appear to be an obvious divergence between solutions.2D 1 rank vs. 4 rank
The following test uses the
examples/2D_moving_lag_bubexample case. The left frame of the video shows the pressure difference between the two simulations, which is identically zero at all time steps. The right pane shows the bubble cloud of the 1- and 4-rank simulations overlaid in black and white, with no obvious divergence visible. The pressure field is shown in the background of the right pane, and the domain decomposition is indicated by the vertical and horizontal lines.https://github.com/user-attachments/assets/173e2ff5-b0a5-4305-9b61-aba2dc4b282a
3D 1 rank vs. 8 rank
The following tests use the
examples/3D_moving_lag_particlesexample case (with some modifications for bubble dynamics and such). The first plot shows the results using particles (bubble_model=0) with identical integral quantities.The next plot shows integral quantities and their relative errors when radial dynamics are enabled. Relative differences are small, though they creep above machine precision.
Checklist
See the developer guide for full coding standards.
GPU changes (expand if you modified
src/simulation/)Remaining items before merge
Must do
vel_model > 0paths (tracer and Newton's 2nd Law) intoolchain/mfc/test/cases.py. The2D_moving_lag_bubsand3D_moving_lag_particlesexamples useadap_dtwith parameters tuned for their full grid size and hang when reduced to 25x25 test grids. Dedicated test cases with appropriate parameters are needed.docs/documentation/case.mdfor new parameters (vel_model,drag_model,pressure_force,gravity_force,write_void_evol,input_path,charNz)Should verify
n_el_bubs_glbis rank-0 only afterMPI_REDUCE— confirmed intentional (only rank 0 prints stats)nReal = 7 + 16*2 + 10*lag_num_tsmagic number ins_initialize_particles_mpi— this buffer size must stay in sync with the pack/unpack loops. Consider computing it from named constants or adding a comment mapping each count to its fieldAlready fixed (by review commits)
Correctness fixes:
xi(2)->xi(1), L(4) numeratorxi(4)->xi(5)(m_bubbles_EL_kernels.fpp)case defaultinf_advance_stepremoved — dereferenced absent optionalfVel/fPosargs when called from EE path withadap_dt(m_bubbles.fpp)particle_in_domain_physicalto match allocation size (m_bubbles_EL.fpp)bub_ppthermodynamic params were inverted (2D_moving_lag_bubs,3D_moving_lag_particles)$:END_GPU_PARALLEL_LOOP()ins_enforce_EL_bubbles_boundary_conditions(m_bubbles_EL.fpp)GPU fixes:
adap_dt_stopadded to GPUprivateclause inm_bubbles_EE.fpp(race condition)MPI fixes:
s_mpi_reduce_int_sum#elsebranch for non-MPI builds (m_mpi_common.fpp)bubs_glbinitialized to 0 before conditional MPI reduce (m_mpi_common.fpp)@:DEALLOCATEforp_send_buff,p_recv_buff,p_send_idswithallocated()guard (m_mpi_proxy.fpp)intent(in)added onlag_num_ts,nBub,pos,posPrevin MPI subroutines (m_mpi_proxy.fpp)Code quality fixes:
hyper_cleaningremoved from pre_process namelist (m_start_up.fpp)3->3._wpin vel_model=3 force computation (m_bubbles.fpp)beta_varsindices documented:1=void fraction, 2=d(beta)/dt, 5=energy source(m_constants.fpp)write_void_evol,pressure_force,gravity_forcein pre_process (m_global_parameters.fpp)sum.pngremoved from examplespathvariable shadowing fixed incases.pyTest fixes:
2D_moving_lag_bubsand3D_moving_lag_particlesadded tocasesToSkip— these examples useadap_dtwith parameters tuned for 200x200 grids. When reduced to 25x25, the CFL condition forces thousands of internal sub-steps, causing hangs. Stale golden files removed.Review commit changelog (sbryngelson/Claude Code)
The following commits were added by @sbryngelson via Claude Code review to fix bugs and bring the PR to CI-passing state. All changes are to wilfonba's code; no new features were added.
52adde37— Fix review bugs: Lagrange interpolation, GPU race, MPI, namelistm_bubbles_EL_kernels.fpp:664):(xi(2) - xi(5))corrected to(xi(1) - xi(5))(pos - xi(4))corrected to(pos - xi(5))— must exclude own indexadap_dt_stopadded toprivateclause inm_bubbles_EE.fppGPU parallel loop#else; sum = var_loctos_mpi_reduce_int_suminm_mpi_common.fppbubs_glb = 0added before conditional MPI reduce in both MPI and non-MPI pathshyper_cleaningfrom pre_processm_start_up.fppexamples/3D_moving_lag_particles/sum.png506dccd2— Fix optional arg UB, missing deallocates, missing intentcase defaultinf_advance_step(m_bubbles.fpp) that wrote to absent optional argsfVel/fPoswhen called from the EE path withadap_dt=T@:DEALLOCATEforp_send_buff,p_recv_buff,p_send_idsins_finalize_mpi_proxy_moduleintent(in)onlag_num_tsins_initialize_particles_mpiandnBub,pos,posPrevins_add_particles_to_transfer_list3->3._wpin vel_model=3 force computationf6adae3a— Fix dealloc crash on single-rank MPIif (bubbles_lagrange)toif (allocated(p_send_buff))— the buffers are only allocated whennum_procs > 1(insides_initialize_particles_mpi), so single-rank MPI runs crashed with "Attempt to DEALLOCATE unallocated"fa2bc4e4— Fix GPU directive, example physics, documentation$:END_GPU_PARALLEL_LOOP()after the locate-cell loop ins_enforce_EL_bubbles_boundary_conditions2D_moving_lag_bubs/case.pyand3D_moving_lag_particles/case.py— all 10bub_ppthermodynamic parameters (gam_v/gam_g,M_v/M_g,k_v/k_g,cp_v/cp_g,R_v/R_g) were inverted vs the correct convention in3D_lagrange_shbubcollapsebeta_varsindices inm_constants.fpp99dbd4d0— Fix restart write OOB and uninitialized lag_paramss_write_restart_lag_bubblesallocated buffer with filtered count (bub_id) but wrote loop iterated over total count (n_el_bubs_loc). Addedparticle_in_domain_physicalfilter to the write loop using a separate counteriwrite_void_evol = .false.,pressure_force = .false.,gravity_force = .false.to pre_processm_global_parameters.fppdefault initializationd030b8a4— Remove pylint directives# pylint: disablecomments fromcases.py(replaced by ruff per upstream lint: add pylint directive check and remove dead directives #1323)c0f63238— Formatting and lint cleanup after rebasepathvariable shadowing lint error incases.py(renamed tocase_path)===separator comment that new source linter flags35a6d5bd— Skip hanging example tests2D_moving_lag_bubsand3D_moving_lag_particlestocasesToSkipincases.pyadap_dt=Twithdt=1on domain[-2000,2000]. When the test framework reduces the grid from 200x200 to 25x25, cell size increases from 20 to 160, but sound speed (~1500 m/s) stays the same. The adaptive stepper gets stuck in thousands of sub-steps trying to satisfy CFL, causing an infinite hang.AI code reviews
Reviews are not triggered automatically. To request a review, comment on the PR:
@coderabbitai review— incremental review (new changes only)@coderabbitai full review— full review from scratch/review— Qodo review/improve— Qodo code suggestions