Skip to content

Let prescribed kinematics drive airfoil and STL immersed boundaries - #1929

Open
sbryngelson wants to merge 5 commits into
MFlowCode:masterfrom
sbryngelson:fix/moving-stl-kinematics
Open

sbryngelson wants to merge 5 commits into
MFlowCode:masterfrom
sbryngelson:fix/moving-stl-kinematics

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

Summary

Lets prescribed kinematics (kin_model) drive airfoils and STL models (geometry 4, 5, 11, 12), and fixes the three bugs that stood in the way. Closes #1895, closes #1896, closes #1902.

For these geometries a moving body's working centroid is moved to the centre of mass of its marked cells, and the difference is kept in centroid_offset (body frame). Four things did not account for that:

  1. Setup deadlock on more than one rank (Moving STL/airfoil IB deadlocks at setup on more than one rank: s_compute_centroid_offset is a collective called from a rank-local loop #1895). s_compute_centroid_offset does MPI_Allreduces but was called from a loop over the rank-local num_ibs, so ranks entered it different numbers of times. It is now called for every global patch id on every rank. A rank that does not hold the patch contributes zeros, and whether a reduction is needed at all is itself reduced, because only holding ranks can see geometry and moving_ibm.
  2. Levelset measured against a phantom body (s_model_levelset does not subtract centroid_offset, so a moving STL's image points are measured against a phantom body #1896). The marker test in s_apply_ib_patches subtracts centroid_offset after rotating into the body frame, but s_model_levelset did not. Image points were therefore placed relative to a body shifted by the offset from the one that was marked. One line.
  3. Kinematics about the wrong point. s_prescribed_kinematics places the centroid at hinge + R·kin_offset, where kin_offset points at the centroid named in the case file. Once that centroid has been moved to the centre of mass, the vector has to be kin_offset - centroid_offset. Otherwise the whole body is displaced by R·centroid_offset.
  4. Restart changes the reference point (Restarting a moving IB re-measures the centre of mass from the body at the restart attitude, so the kinematics change reference point across a restart #1902). On restart the offset was re-measured from the body voxelised at the restart attitude, which differs from the one measured at t = 0. The offsets are now written with each checkpoint (restart_data/ib_offset_<step>.dat) and read back on restart. If no file is present, the measured offsets stand as before.

The validator's prohibition of kin_model on geometries 4, 5, 11 and 12 is removed, and case.md documents the convention. Static bodies and kin_model on other geometries are unaffected: their offset is zero.

Verification

  • pre_process, simulation and post_process build on master 0f7f62fa with this branch in all three Frontier configurations: CPU (--no-gpu), OpenMP offload (--gpu mp) and OpenACC (--gpu acc).
  • ./mfc.sh precheck passes (all 7 gates).
  • IBM tests (CPU, run as CI runs them on Frontier): 59 of 60 pass. The one that did not run, 2D -> IBM -> Vieille Burn Rate, needs a chemistry build that was not present locally; CI builds it.

A flapping STL plate under kin_model = 1, with its case-file centroid 0.02 off the plate's centre of mass so that centroid_offset is nonzero. Master's validator rejects this case, and before this PR it deadlocked at setup on more than one rank.

Check Result
1 vs 2 ranks (64×32×32, 399 steps) force, torque and centroid path bit-identical; ib_offset_200.dat identical
4 vs 8 ranks (100×50×50, 399 steps) force and centroid path bit-identical
8 ranks, restart at step 200 with the saved offsets centroid path and torque identical to the uninterrupted run
8 ranks, restart at step 200 with ib_offset_200.dat removed reported centroid jumps by 3.6e-3 (0.18 cells) and torque by 2.6% at the first step after restart

After a restart the body itself is placed identically either way, because the marker test and the kinematics now both use kin_offset - centroid_offset, so the offset cancels in the geometry. What the saved offsets preserve is the reference point for the centroid and the torque, which is what #1902 reports.

Forces after a restart match the uninterrupted run to all printed digits for about 20 steps, then differ at the 1e-3 level as cells cross the moving surface. They do that equally with and without the offset file: it is the round-off sensitivity of a restart, which this PR does not change.

The same fixes carried an STL seagull with prescribed flapping on up to 64 Frontier nodes, over six wingbeats across chained restarts.

AI disclosure

Written with Claude Code (Anthropic). These fixes come from running an STL seagull with prescribed flapping on Frontier (up to 64 nodes, six wingbeats across chained restarts), and were reduced from the fixes used there.

Acknowledgement

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

These geometries move their working centroid to the centre of mass and keep the difference in
centroid_offset. Account for it everywhere it matters:

- s_compute_centroid_offset is collective; call it for every global patch on every rank (MFlowCode#1895)
- s_model_levelset subtracts centroid_offset, matching the marker test (MFlowCode#1896)
- prescribed kinematics rotate kin_offset - centroid_offset about the hinge
- checkpoints record the offsets and restarts restore them (MFlowCode#1902)

The validator no longer forbids kin_model on geometries 4, 5, 11 and 12.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 09:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 13.69863% with 63 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.69%. Comparing base (ed7a238) to head (28694fc).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/simulation/m_ibm.fpp 11.53% 45 Missing and 1 partial ⚠️
src/simulation/m_data_output.fpp 15.00% 16 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1929      +/-   ##
==========================================
- Coverage   62.82%   62.69%   -0.14%     
==========================================
  Files          86       86              
  Lines       22394    22447      +53     
  Branches     3305     3324      +19     
==========================================
+ Hits        14070    14074       +4     
- Misses       6071     6118      +47     
- Partials     2253     2255       +2     

☔ 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.

Only moving airfoils and STLs carry a centroid_offset, but setup ran one collective per global patch and each
checkpoint three, so particle clouds (thousands of patches) paid for work they never use. One reduction now
decides whether any patch needs it; otherwise nothing runs and no ib_offset file is written. The checkpoint
writer also reduces in a single call, with only each patch's owner contributing.

Co-Authored-By: Claude <noreply@anthropic.com>
@sbryngelson

Copy link
Copy Markdown
Member Author

Pushed 42f38f1a. CI's Ubuntu Intel (no-debug) lane crashed in the five particle-cloud tests (malloc(): invalid size at the final checkpoint), on this PR only. Particle clouds never need a centroid offset, but setup ran one collective per global patch and each checkpoint three, over thousands of patches. One reduction now decides whether any patch needs an offset; otherwise none of this runs and particle-cloud runs take master's path exactly. The checkpoint writer also reduces in a single call.

The five tests pass on Frontier under GNU with -fcheck=all and under AddressSanitizer, both before and after this commit, so I could not reproduce the Intel crash itself; CI's Intel lane is the check. The moving-STL verification in the description is bit-identical before and after this commit, including ib_offset_200.dat.

Conflict in s_ibm_setup: master's s_check_every_patch_marked (MFlowCode#1914) runs first, then this branch's gated
centroid-offset work.
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/simulation/m_ibm.fpp 1485 +46
src/simulation/m_data_output.fpp 1570 +28
src/simulation/m_compute_levelset.fpp 454 +1
Directory Lines Diff
simulation 28051 +75
total 46995 +75

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

2 participants