Skip to content

Ib collisions#1348

Draft
danieljvickers wants to merge 53 commits intoMFlowCode:masterfrom
danieljvickers:ib-collisions
Draft

Ib collisions#1348
danieljvickers wants to merge 53 commits intoMFlowCode:masterfrom
danieljvickers:ib-collisions

Conversation

@danieljvickers
Copy link
Copy Markdown
Member

Description

Added a soft-sphere collision model for sphere/circle IB patches to collide with each other and walls. Collisions with friction were validated against references to obtain reasonable results. This code was also run on 64 MPI ranks to simulate a 10k particle case in a shock tube. Adding this feature also required the ability to restart from previous data, which was added and integrated with existing file reading. That required some refactoring of the existing IB state writing, and will require more modifications to write in parallel.

Fixes #1108

Type of change

  • New feature
  • Refactor
  • Documentation
  • Other: describe

Testing

Ran a 10k particle case as well as the new example to compare rebound angles and collision energies.

Checklist

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

See the developer guide for full coding standards.

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
  • @claude full review — Claude full review (also triggers on PR open/reopen/ready)
  • Add label claude-full-review — Claude full review via label

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

Claude Code Review

Head SHA: 0e3fca9

Files changed:

  • 20
  • src/common/m_constants.fpp
  • src/simulation/m_collisions.fpp
  • src/simulation/m_global_parameters.fpp
  • src/simulation/m_ibm.fpp
  • src/simulation/m_ib_patches.fpp
  • src/simulation/m_start_up.fpp
  • src/simulation/m_data_output.fpp
  • src/pre_process/m_global_parameters.fpp
  • toolchain/mfc/params/definitions.py
  • toolchain/mfc/case_validator.py

Findings:

[Critical] num_patches_max reduced from 1000 to 10 — breaks all simulations with >10 IC patches

src/common/m_constants.fpp: num_patches_max is reduced from 1000 to 10. This constant bounds patch_icpp arrays throughout the code (pre_process, simulation, post_process). Any existing case file using more than 10 patch_icpp(i) blocks will now silently access out-of-bounds memory or fail at runtime. This is a wide blast-radius regression — the old limit existed precisely because users of pre_process need many patches for complex geometries. The split into num_patches_max (IC patches) and num_ib_patches_max (IB patches) is sensible, but 10 is far too small for patch_icpp.


[High] ib_coefficient_of_friction missing from collision_model requires-constraint — silent dflt_real forces

toolchain/mfc/params/definitions.py (collision_model constraint):

"collision_model": {
    "when_set": {
        "requires": ["ib", "coefficient_of_restitution", "collision_time"],
    }
},

ib_coefficient_of_friction is not listed as required. In src/simulation/m_collisions.fpp it is used directly: tangental_force = -ib_coefficient_of_friction * norm2(normal_force) * tangental_vector. The default for ib_coefficient_of_friction is dflt_real = -1e6_wp (see src/simulation/m_global_parameters.fpp). A user who sets collision_model = 1 without setting ib_coefficient_of_friction will silently produce tangential forces scaled by −10⁶, corrupting the collision physics. ib_coefficient_of_friction must be added to the requires list.


[High] patch_ib IndexedFamily max_index raised to 50,000 — Python registry performance regression

toolchain/mfc/params/definitions.py:

-            max_index=NUM_PATCHES_MAX,   # was 1000
+            max_index=NIB,               # now 50000

The comment directly above this line states: "max_index is None so the parameter registry stays compact (no enumeration)". The previous value of NUM_PATCHES_MAX was 1000; the new NIB is 50,000 — 50× more entries for the patch_ib(i)%attr family. Every ./mfc.sh validate, ./mfc.sh params, and test invocation will pay this enumeration cost. This change contradicts the documented intent of keeping the registry compact.


[High] s_read_ib_restart_data contains bare print * debug statements

src/simulation/m_start_up.fpp (new s_read_ib_restart_data subroutine, called on rank 0 at restart):

print *, "IB Restart File Exists: ", file_exist
print *, "iostat ", ios

Per project convention, debug output must use @:LOG(...) (active only in MFC_DEBUG builds) or be removed. Bare print * statements pollute production stdout on every restarted simulation.


[Medium] case_validator.py fallback for num_ib_patches_max mismatches Fortran constant

toolchain/mfc/case_validator.py:

num_ib_patches_max = get_fortran_constants().get("num_ib_patches_max", 100000)

The Fortran constant is 50000 (src/common/m_constants.fpp). The fallback of 100000 would allow twice as many IBs as Fortran can accommodate when get_fortran_constants() fails to parse the file. The fallback should be 50000.


[Low] Stray Doxygen group tag on ib_bc_z declaration

src/simulation/m_global_parameters.fpp:

$:GPU_DECLARE(create='[ib_bc_x, ib_bc_y, ib_bc_z]')
!> @}ib_bc_z

The closing group tag should be !> @} (no identifier suffix). The ib_bc_z suffix is not valid Doxygen syntax.

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.

MIBM Restart Data Support

1 participant