IBM force calculation simplification (Dr Bala recommendations)

[mrvandenboom]

User description

(cherry picked from commit 2a034dc) (cherry picked from commit 1d8b109)

Description

This change takes advantage of the symmetric and asymmetric attributes of the tensors to simplify the calculation of force and torque on immersed boundaries.

Type of change

• Bug fix
• New feature
• [ X] Refactor
• Documentation
• Other: describe

Testing

ran ./mfc.sh lint
ran ./mfc.sh format
ran ./mfc.sh test -a on Hipergator (CPU). All tests passed.

Summary by CodeRabbit

• Refactor
  • Reorganized internal simulation computation logic for more efficient handling of force and torque calculations by consolidating updates and reducing redundant operations.

---

CodeAnt-AI Description

Compute torque from combined pressure and viscous forces and reduce atomic updates

What Changed

• Torque is computed once from the final local force (pressure + viscous) instead of from many intermediate cross-product terms
• Viscous contributions are added directly to the force divergence rows before torque is computed, ensuring torque reflects combined pressure and viscous effects
• Global force and torque arrays are updated atomically only after each point's force and torque are fully assembled, reducing repeated atomic writes

Impact

✅ Correct torque from combined pressure and viscous contributions
✅ Fewer atomic updates during force/torque assembly
✅ Lower contention during per-point force assembly

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

[codeant-ai]

Nitpicks 🔍

🔒 No security issues identified

⚡ Recommended areas for review Torque correctness The PR changes torque assembly: it no longer computes derivatives of the cross-product of the viscous stress (previous approach) and instead computes the cross-product once from the final, combined local force contribution. This is mathematically valid (torque = r x F) but can produce subtle numerical differences compared to the old per-component cross-derivative method. Verify equivalence across test cases (especially near boundaries and with non-uniform viscosity) and check that s_compute_viscous_stress_tensor returns the tensor rows in the ordering assumed here. Atomic update / precision check Forces and torques are updated using GPU atomics. Ensure the target GPU atomic implementation supports the required precision (real(wp)) and that the $:GPU_ATOMIC pragmas correctly protect concurrent updates for both forces and torques. Also validate that local accumulators are private in the parallel region on all backends.

[Copilot]

Pull request overview

Refactors immersed-boundary (IBM) force/torque accumulation in simulation to simplify the force/torque calculation by leveraging tensor symmetry/asymmetry characteristics, per Dr. Bala’s recommendations.

Changes:

• Simplifies viscous-force contribution assembly by removing per-component cross-product derivatives.
• Computes torque via a single r × F cross product after all force contributions are accumulated.
• Reorders/clarifies some inline comments around viscous contribution and atomic updates.

[Copilot]

Inline comment is incorrect: this statement computes the z-derivative of the third row (index 3) of the viscous stress tensor, not the second row. Please update the comment to avoid confusion for future maintenance.

Suggested change

	viscous_stress_div(3, 1:3) = (viscous_stress_div_2(3, 1:3) - viscous_stress_div_1(3, 1:3))/(2._wp*dz) ! get z derivative of the second-row of viscous stress tensor
	viscous_stress_div(3, 1:3) = (viscous_stress_div_2(3, 1:3) - viscous_stress_div_1(3, 1:3))/(2._wp*dz) ! get z derivative of the third-row of viscous stress tensor

[Copilot]

This comment says only the force is updated atomically, but the loop performs atomic updates for both forces and torques. Consider updating the comment to reflect both arrays to prevent misleading documentation.

Suggested change

	! Update the force values atomically to prevent race conditions
	! Update the force and torque values atomically to prevent race conditions

[cubic-dev-ai]

No issues found across 1 file

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/simulation/m_ibm.fpp`:
- Line 1130: Remove the trailing whitespace on the blank line flagged by CI (a
line that contains only spaces) in the m_ibm.fpp source so the formatter passes;
locate the empty/blank line in the file, delete the trailing spaces (convert it
to a truly empty line), run ./mfc.sh format to verify no formatting diffs
remain, and commit the change.

[codecov]

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.06%. Comparing base (df28255) to head (24f27a2).
⚠️ Report is 29 commits behind head on master.

Files with missing lines	Patch %	Lines
src/simulation/m_ibm.fpp	71.42%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1234   +/-   ##
=======================================
  Coverage   44.05%   44.06%           
=======================================
  Files          70       70           
  Lines       20496    20481   -15     
  Branches     1989     1989           
=======================================
- Hits         9030     9024    -6     
+ Misses      10328    10319    -9     
  Partials     1138     1138           

☔ View full report in Codecov by Sentry.
📢 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.

[github-actions]

Claude Code Review

1 file changed: src/simulation/m_ibm.fpp

Summary

• Refactors s_compute_ib_forces in m_ibm.fpp to simplify the torque calculation from a per-direction cross-product accumulation to a single cross product applied to the fully-assembled force vector.
• The mathematical equivalence holds by linearity of the cross product: r × F_p + r × F_v = r × (F_p + F_v), so computing torque from the combined force after adding viscous contributions is equivalent to the old per-direction approach.
• Eliminates ~6 cross-product calls per IB grid point per time step in 3D viscous cases, plus removes the explicit loop over rows l — a meaningful GPU compute savings on dense IBM meshes.
• The new code narrows each viscous_stress_div update from a full 3×3 matrix write to a single-row write per direction, which is a cleaner representation of ∇·σ.
• PR states CPU tests passed on HiPerGator; no GPU test run is mentioned.

---

Findings

Issue 1 — Dead variables left in declaration and GPU private clause (m_ibm.fpp)

viscous_cross_1 and viscous_cross_2 are no longer assigned or read anywhere in the new code, but they remain in two places:

Line ~1053 (variable declaration):

real(wp), dimension(1:3, 1:3) :: viscous_stress_div, viscous_stress_div_1, viscous_stress_div_2, viscous_cross_1, viscous_cross_2

Line ~1074 (GPU_PARALLEL_LOOP private clause):

$:GPU_PARALLEL_LOOP(private='[... viscous_cross_1, viscous_cross_2, ...]', ...)

These should be removed. On GPU builds the unused private allocations are harmless but waste per-thread stack space, and they create misleading code. The l loop variable can also be examined — it is still used in the atomic update loop below, so it stays.

Recommendation: Remove viscous_cross_1 and viscous_cross_2 from both the declaration and the private list.

---

Issue 2 — Incorrect comment on z-direction block (m_ibm.fpp)

The new z-direction line says "second-row" but indexes row 3:

viscous_stress_div(3, 1:3) = (viscous_stress_div_2(3, 1:3) - viscous_stress_div_1(3, 1:3))/(2._wp*dz) ! get z derivative of the second-row of viscous stress tensor

This is a copy-paste error from the y-direction comment. It should read "third-row".

---

Issue 3 — GPU test coverage not confirmed

This PR modifies code in src/simulation/ inside a GPU parallel loop (GPU_PARALLEL_LOOP). The PR description notes only CPU tests on HiPerGator. For a change to GPU-offloaded IBM force/torque assembly, a GPU regression run (e.g., on Phoenix or HiPerGator GPU partition) would close the loop, since CPU and GPU path divergences are possible (e.g., private variable scoping, atomics behavior).

---

Improvement Opportunities

1. Remove the ! TODO :: This is really bad code comment if this refactor resolves the concern it referred to, or update it to reflect what remains suboptimal about the viscous stress stencil approach.

2. Unify the y-block comment style — the x-block says "first-row", y says "second-row", z (after fix) would say "third-row". Consider a single parametric comment pattern or a brief note above the block explaining the ∇·σ assembly pattern to make the index-to-direction mapping self-evident.

3. Consider extracting the viscous force divergence assembly into a helper subroutine (e.g., s_compute_viscous_force_divergence) to reduce the per-dimension repetition and make s_compute_ib_forces easier to read — the current TODO comment suggests this was already considered.