Repository navigation
Conversation
Motivation Moving immersed boundaries can receive NaN or incorrectly scaled inertia, and small nonzero angular momentum can be mistaken for zero. Expectation Compute physical inertia after cell marking and preserve the direction of every finite nonzero rotation axis. Change Four errors in how a two-way-coupled IB's moment of inertia is set: - s_ibm_setup computed it before the IB markers were filled. Shapes without a closed form (airfoil, STL, cuboid, cylinder, ...) sum over the body's marked cells, found none, and stored 0/0 = NaN, so the run aborted. It is now computed once the markers and the centre-of-mass offsets are in place. - The cell sum divided by count*cell_volume. With patch%mass the total mass, each of the count cells carries mass/count, so the moment is mass*sum(r^2)/count; the extra 1/cell_volume made it grid dependent. - The rectangle used m*(lx^2 + ly^2)/6. length_x and length_y are full side lengths, so the moment about the centroid is /12, the same convention the ellipse branch already uses. - In 3D the time stepper passes the angular momentum as the axis, and its norm was compared with sgm_eps = 1e-16, a threshold with units. A cuboid with I = 6e-14 kg m^2 spinning at 1e-3 rad/s got the dummy moment 1, and the next RK stage took omega_z to 8.3e9 rad/s. Only the exact zero vector now counts as not rotating; the axis is scaled by its largest component before normalising so it cannot underflow. A rank holding no cell of the body now gets the dummy moment 1 instead of 0/0, the same guard and value as MFlowCode#1992. Circles, spheres, and prescribed-motion or static IBs are unchanged; no goldens move. Result The corrected initialization, formulas and axis normalization are retained. The precheck passes and regression reference files are unchanged. Rank-local sums and nonuniform-cell weighting remain limitations. Fixes MFlowCode#1994 Co-authored-by: ruru25254649 <ruru.2525.4649@gmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WPhkY8p1EHhGY7jScizJmC
Lines of Code
|
|
Claude Code Review Head SHA: c0a2c4e Files changed:
Findings:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2109 +/- ##
==========================================
+ Coverage 61.95% 61.98% +0.02%
==========================================
Files 86 86
Lines 22906 22908 +2
Branches 3378 3378
==========================================
+ Hits 14192 14200 +8
+ Misses 6206 6200 -6
Partials 2508 2508 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
claude code review isn't happy with this one |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1994.
Two-way-coupled immersed boundaries (
moving_ibm = 2) get a wrong moment of inertia:This PR computes the moment after the body's cells are marked, fixes the two formula errors, and fixes the zero-rotation check. Circles, spheres, and prescribed-motion or static IBs are unchanged. No goldens change.
Changes (all in
src/simulation/m_ibm.fpp)s_ibm_setupcalleds_compute_moment_of_inertiain its first loop, befores_apply_ib_patchesfillsib_markers. Shapes without a closed form sumr²over the body's marked cells. At that point there are none, so they got0/0 = NaN.s_propagate_immersed_boundariesmultiplies by the stored value before recomputing it.cell_volumefrom the cell sum.patch%massis the body's total mass: it is used asforce/mass, and the analytic branches use0.4*mass*r².countcells therefore carriesmass/count, so the moment ismass*Σr²/count.count*cell_volumegave the wrong units and made the moment scale with the grid./12, not/6.length_xandlength_yare full side lengths:f_is_inside_cuboidtests|x| ≤ length_x/2, and the ellipse branch uses the same convention.m(lx² + ly²)/12.0/0on a rank holding no cell of the body. It returns the dummy value 1, the same guard and value as Decode periodic IB markers in the moment of inertia sum #1992 (the non-rotating case already returns 1).sgm_eps = 1e-16, a threshold with units, so a small, slowly spinning body got the dummy moment 1.Side effects
s_ibm_setup. Nothing between the old and new positions readspatch_ib%moment: its only readers are the time stepper and the output.Scope, and relation to #1992
#1992 (closed, tracked in
pr-to-do#2068) decodes periodic markers in this sum and adds thecount > 0guard. This PR uses the same guard. The two PRs still overlap in this routine on thecell_volumelines (declaration, computation, final division), and a rebase drops them.#1992 alone does not fix point 1. The setup-time call would still find no marked cells, so every cell-sum body would get the dummy moment of 1. For the airfoil below that is 1 kg·m² instead of 5.7e-14, so the body could barely rotate.
Not changed here, as noted in #1994 and #2068:
GPU_ROUTINEcalled inside device loops.s_compute_centroid_offset) would need volume weighting. That fits with the host-side rewrite above, rather than in this PR.Verification
I printed
patch_ib(1)%momentat the end ofs_ibm_setupwith a temporary print, not part of the PR. Setup:mass = 1e-6,moving_ibm = 2.examples/2D_mibm_cylinder_in_cross_flowandexamples/2D_ibm_airfoil.icfl = NaN)MPI_ABORT)./mfc.sh test -o IBM), final branch, GNU 13.3 + Open MPI 4.1.609FDDC81) has no torque, so its golden does not movecount > 0guard, which only matters with MPI)--gpu acc, H100 (before the guard)The axis check (change 5).
examples/3D_rotating_spherewith the sphere replaced by a 0.8 × 0.4 × 0.6 mm cuboid,moving_ibm = 2,mass = 1e-6,angular_vel(3) = 1e-3, still fluid, inviscid, 50³ grid, 1 rank.The
count > 0guard on 4 ranks.moment = NaN. With it, they carry 1.Contribution Policy
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Acknowledgement
This PR is from Diablo (Tsinghua University) participating in the SC26 Student Cluster Competition.