Skip to content

1454-projection-optimization #1549

Draft
danieljvickers wants to merge 1 commit into
MFlowCode:masterfrom
danieljvickers:1454-projection-optimization
Draft

1454-projection-optimization #1549
danieljvickers wants to merge 1 commit into
MFlowCode:masterfrom
danieljvickers:1454-projection-optimization

Conversation

@danieljvickers

@danieljvickers danieljvickers commented Jun 9, 2026

Copy link
Copy Markdown
Member

Description

In many-particle cases, the limiting factor in the IBM compute is by far the time it takes to project the immersed boundaries onto the grid. This is fundamentally rooted in how we parallelize the work. The current code parallelizes of x, y, and z, but sequentially iterates through the IB patches. In cases where there are many IBs that are small, we are launching several (thousands) of GPU kernels each time step, but each kernel only works on hundreds of grid cells. Adding an option to parallelize over the thousands of particles should be significantly larger parallelism and thus optimize the projection.

In order to maintain the functionality of both parallelism methods, I need a separate set of geometry bounding checks. Since we perform a check in icpp patches and now 2 in IB patches, this is a significant amount of redundant code that must be maintained. To be somewhat forward-looking, I opted to merge all geometry checking into a single module that can be called from both forms of IB parallelism and the icpp pre_processing code. This should clean up the code nicely and significantly reduce code maintenance going forward. Since I am apparently refactoring everything again, I also have opted to update the patch geometry indexes to use a more unified framework and deprecate the unneeded cylinder length checks.

The end result is the creation of a new module in common, the deletion of duplicate code, and a new parallelism path for IB patches when utilizing GPU compute.

Closes #1454, #1532, #1543

Type of change (delete unused ones)

  • New feature
  • Refactor

Testing

Not Yet.

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 Jun 9, 2026

Copy link
Copy Markdown

Claude Code Review

Head SHA: f85f983

Files changed:

  • 6
  • examples/2D_mibm_particle_cloud/case.py
  • src/common/m_patch_geometries.fpp
  • src/simulation/m_checker.fpp
  • src/simulation/m_global_parameters.fpp
  • src/simulation/m_ibm.fpp
  • toolchain/mfc/params/definitions.py

Findings:

[Correctness — Compilation failure] f_is_inside_cuboid: spurious line-continuation & breaks the assignment

src/common/m_patch_geometries.fpp, added lines ~97–98:

is_inside = -0.5_wp*patch_ib(patch_id)%length_x <= x .and. 0.5_wp*patch_ib(patch_id)%length_x >= x &
is_inside = is_inside .and. -0.5_wp*patch_ib(patch_id)%length_y <= y.and. ...

The trailing & on the first line causes the second line to be treated as a continuation of the first assignment, producing the invalid token sequence ... >= x is_inside = is_inside .... All four CI-gated compilers will reject this. The & must be removed so the two assignments are separate statements.


[Correctness — Compilation failure] f_is_inside_rectangle and f_is_inside_airfoil_3D declared public but never defined

src/common/m_patch_geometries.fpp, module public list:

public :: f_is_inside_circle, f_is_inside_sphere, f_is_inside_cylinder, f_is_inside_rectangle, f_is_inside_cuboid, &
    & f_is_inside_airfoil, f_is_inside_airfoil_3D, f_is_inside_ellipse

Neither f_is_inside_rectangle nor f_is_inside_airfoil_3D has a corresponding function body anywhere in the new module. Naming undefined symbols in a public statement is a compilation error.


[Correctness — Undefined behaviour] f_is_inside_ellipse has no body; returns uninitialized logical

src/common/m_patch_geometries.fpp, added lines ~165–172:

function f_is_inside_ellipse(patch_id, x, y) result(is_inside)
  ...
  logical :: is_inside

end function f_is_inside_ellipse

is_inside is never assigned. Every call site will silently receive an indeterminate value, producing wrong inside/outside classifications without any diagnostic.

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.

Projection Optimization

1 participant