Skip to content

Large-Scale Startup Optimizations - #1686

Open
danieljvickers wants to merge 7 commits into
MFlowCode:masterfrom
danieljvickers:smarter-particle-bed-instantiation
Open

Large-Scale Startup Optimizations#1686
danieljvickers wants to merge 7 commits into
MFlowCode:masterfrom
danieljvickers:smarter-particle-bed-instantiation

Conversation

@danieljvickers

Copy link
Copy Markdown
Member

Description

I performed a weak scaling test to the full of OLCF Frontier. To get the simulation to run correctly, the initialization process in simulation had to be modified to accommodate the number of IBs in the simulation as well as specific slowdowns I encountered at the full system size. This branch contains those modifications.

Type of change (delete unused ones)

  • Bug fix
  • Refactor

Testing

Ran 500 million immersed boundaries on 78k MPI ranks.

Checklist

Check these like this [x] to indicate which of the below applies.

  • I added or updated tests for new behavior
  • I updated documentation if user-facing behavior changed

See the developer guide for full coding standards.

GPU changes (expand if you modified src/simulation/)
  • GPU results match CPU results
  • Tested on NVIDIA GPU or AMD GPU

AI code reviews

Reviews are not retriggered automatically. To request a review, comment on the PR:

  • @claude full review — Claude full review (also triggers on PR open/reopen/ready)
  • Or add label claude-full-review — Claude full review via label

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

Claude Code Review

Head SHA: f35421e

Files changed:

  • 8
  • src/common/m_boundary_io.fpp
  • src/common/m_constants.fpp
  • src/common/m_delay_file_access.f90
  • src/pre_process/m_data_output.fpp
  • src/simulation/m_collisions.fpp
  • src/simulation/m_data_output.fpp
  • src/simulation/m_particle_cloud.fpp
  • src/simulation/m_start_up.fpp

Findings:

  • src/simulation/m_collisions.fpp: the active IB collision routine is swapped from s_detect_ib_collisions_n2 to s_detect_ib_collisions (the previous call is commented out instead). This is a behavior/physics change unrelated to the rest of the PR's particle-cloud refactor, and it lands in the same commit as a 15x increase to num_local_ibs_max (2000 → 30000 in m_constants.fpp) with no explanation of why the alternate collision algorithm is now preferred at that scale. Please confirm this swap is intentional and not a leftover from local debugging before merging, since it silently changes which collision-detection path every IB simulation takes.
  • src/common/m_constants.fpp: num_ib_patches_max_namelist jumps 15x (54000 → 810000). Per its own docstring this is the "Fixed capacity of patch_ib ... patch_ib is never reallocated", i.e. a fixed-size ib_patch_parameters array allocated on every MPI rank regardless of how many IBs that rank actually holds (num_local_ibs_max is only raised to 30000, 27x smaller). If ib_patch_parameters embeds any of the STL/airfoil fixed-size fields sized off num_stl_models_max/num_ib_airfoils_max, this multiplies per-rank memory for patch_ib by ~15x across every rank in an exascale run. Worth confirming this capacity was deliberately sized (not just copied up alongside the particle-cloud limits) given the per-rank memory cost.

@danieljvickers
danieljvickers marked this pull request as ready for review July 30, 2026 02:58
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