Invicid two-way fluid-structure interaction #1075
Conversation
…is accelerating. Two is it is going uupward randomly. And three is it has a very large pressure interpolation.
…sectino to another in an array
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds two-way immersed-boundary coupling: new IB mass/moment/force/torque fields, MPI vector all-reduce and broadcast support, pressure-based IB force/torque computation and moment-of-inertia estimation, vectorized RK IB updates (one- and two-way), an example case, docs/tooling tweaks, and new golden test data. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant TS as TimeStepper
participant IBM as m_ibm
participant Fluid as FluidState (pressure)
participant MPI as MPI Layer
participant Patch as patch_ib
TS->>IBM: RK substep: advance IB state
IBM->>Fluid: sample pressure at IB markers
IBM->>IBM: integrate pressure → local force & torque
IBM->>MPI: s_mpi_allreduce_vectors_sum(force_loc, force_glb, nvec, 3)
MPI->>Patch: broadcast/reduce global force & torque (mass included)
alt moving_ibm == 2 (two-way)
IBM->>IBM: s_compute_moment_of_inertia(geometry)
IBM->>Patch: vel += (force / mass) * dt
IBM->>Patch: angular_vel += (torque / moment) * dt
else moving_ibm == 1 (one-way)
IBM->>Patch: update kinematics without feedback
end
IBM->>Patch: angles += angular_vel * dt
Patch->>TS: return updated IB state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
codeant-ai
bot
added
the
size:XL
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| @@ -0,0 +1,105 @@ | |||
| import json | |||
| import math | |||
There was a problem hiding this comment.
Suggestion: Unused import: math is imported but never used anywhere; keep dead imports out of the codebase to avoid confusion and maintain clean dependencies — remove the import. [possible bug]
Severity Level: Critical 🚨
| import math | |
| # removed unused import 'math' |
Why it matters? ⭐
The module imports math but never uses it anywhere in the current PR diff. Removing the import removes dead code and linter warnings and has no functional impact.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** examples/2D_mibm_cylinder_in_cross_flow/case.py
**Line:** 2:2
**Comment:**
*Possible Bug: Unused import: `math` is imported but never used anywhere; keep dead imports out of the codebase to avoid confusion and maintain clean dependencies — remove the import.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
toolchain/bootstrap/modules.sh (1)
38-48: Fix interactive selection guard and keep HiPerGator option consistent with help/prompt
- Line 39:
if [ -v $u_c ]; thenis almost certainly wrong.-vexpects a variable name, not its value, and this will misbehave (or error) whenu_cis unset or holds a system code. If the goal is "prompt only when-c/--computerwas not supplied", use something like:if [ -z "${u_c+x}" ]; then # or simply: if [ -z "$u_c" ]; then ... fi
- Lines 11–14 vs 41–48: the new Florida/HiPerGator
(h)choice is in the interactive menu but not in theshow_help“Options” list. Please addFlorida (h)there so CLI help matches the interactive options.- Line 48: the colorized prompt string has a lot of adjacent quoted segments (
"$G""a$W"...), which triggers SC2027. Behavior is fine, but consider simplifying for readability and linters, e.g.:log_n "(${G}a${W}/${G}f${W}/${G}s${W}/${G}w${W}/${C}b${W}/${C}e${CR}/.../${OR}h${CR}): "
🧹 Nitpick comments (5)
src/common/m_derived_types.fpp (1)
335-336: Consider adding documentation comments for new fields.The new fields
mass,moment,force, andtorquelack documentation. Following the codebase style (e.g.,!< Description), consider adding brief descriptions:!! Patch conditions for moving imersed boundaries integer :: moving_ibm ! 0 for no moving, 1 for moving, 2 for moving on forced path -real(wp) :: mass, moment ! mass and moment of inertia of object used to compute forces in 2-way coupling -real(wp), dimension(1:3) :: force, torque ! vectors for the computed force and torque values applied to an IB +real(wp) :: mass !< Mass of IB object for two-way coupling +real(wp) :: moment !< Moment of inertia about rotation axis for two-way coupling +real(wp), dimension(1:3) :: force !< Computed pressure force vector on IB +real(wp), dimension(1:3) :: torque !< Computed torque vector on IB (in local coordinates)src/simulation/m_ibm.fpp (1)
199-212: Missing GPU parallelization for interior pressure loop.This triple-nested loop over the grid is not wrapped with GPU parallel directives, unlike similar loops in this file. For large grids, this could cause performance issues.
! set the Moving IBM interior Pressure Values do patch_id = 1, num_ibs if (patch_ib(patch_id)%moving_ibm == 2) then + $:GPU_PARALLEL_LOOP(collapse=3, private='[j,k,l]') do j = 0, m do k = 0, n do l = 0, p if (ib_markers%sf(j, k, l) == patch_id) then q_prim_vf(E_idx)%sf(j, k, l) = 1._wp end if end do end do end do + $:END_GPU_PARALLEL_LOOP() end if end doexamples/2D_mibm_cylinder_in_cross_flow/case.py (1)
1-103: Case setup matches intended 2D moving-cylinder demo; only minor cleanups
- The dictionary entries for the domain, single fluid, rectangular IC patch, and circular
patch_ib(1)(withmoving_ibm = 2, downwardvel(2), andmass) are consistent with a 2D cylinder-in-cross-flow example and with the new IBM mass field.Muand themathimport are currently unused; you can drop them or, if desired, computeRefromMuto keep the case self-consistent.- Please double-check that
moving_ibm = 2is the correct flag for the new two-way pressure-driven coupling mode, and that this value is documented alongside the other options indocs/documentation/case.md.src/common/m_mpi_common.fpp (1)
475-497: Add shape assertions and clarify alias handling ins_mpi_allreduce_vectors_sumThe MPI call pattern is fine, but two improvements would make this safer and more self-documenting:
- Add Fypp
ASSERTchecks so misuse is caught early, e.g.:@:ASSERT(size(var_loc, 1) == num_vectors .and. size(var_loc, 2) == vector_length, & 's_mpi_allreduce_vectors_sum: shape/length mismatch for var_loc') @:ASSERT(size(var_glb, 1) == num_vectors .and. size(var_glb, 2) == vector_length, & 's_mpi_allreduce_vectors_sum: shape/length mismatch for var_glb')This guards against passing slices or mismatched dimensions while still using packed
num_vectors*vector_lengthin the MPI call.
- The
loc(var_loc) == loc(var_glb)in-place detection relies on the non-standardlocintrinsic and on aliasing two distinct dummy arguments, which is technically non-conforming Fortran. If you don’t strictly need in-place semantics here, consider requiring distinct arrays and dropping the alias path; otherwise, document that this routine assumes compilers withlocand acceptscall s_mpi_allreduce_vectors_sum(A, A, ...)as a deliberate extension.docs/documentation/case.md (1)
378-381: Clean up table formatting (stray.column, hard tabs, and missing trailing pipes)Several of the modified tables now have small formatting issues that markdownlint is flagging:
- Simulation algorithm table header and first row (around lines 378–381) include an extra
.in the first column (| Parameter . | ...and| `bc_[x,y,z]%%beg[end]` . | ...).- The formatted-output and ensemble-averaged bubble model tables (lines ~578–615 and ~781–798), and the patch types table (lines ~1038–1059) still contain hard tabs and at least one line missing a trailing pipe, which triggers MD010/MD055/MD037.
All of this is cosmetic, but it’s easy to fix by:
- Removing the stray
.tokens from the first column,- Replacing tabs with spaces, and
- Ensuring each table row has a consistent set of leading and trailing
|characters.This will keep the docs rendering cleanly and quiet markdownlint.
Also applies to: 578-615, 781-798, 1038-1059
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/documentation/case.md(7 hunks)examples/2D_mibm_cylinder_in_cross_flow/case.py(1 hunks)src/common/m_derived_types.fpp(1 hunks)src/common/m_mpi_common.fpp(1 hunks)src/pre_process/m_global_parameters.fpp(1 hunks)src/simulation/m_ibm.fpp(4 hunks)src/simulation/m_mpi_proxy.fpp(1 hunks)src/simulation/m_time_steppers.fpp(2 hunks)tests/127A967A/golden-metadata.txt(1 hunks)tests/127A967A/golden.txt(1 hunks)toolchain/bootstrap/modules.sh(1 hunks)toolchain/mfc/run/case_dicts.py(2 hunks)toolchain/util.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{fpp,f90}: Use 2-space indentation; continuation lines align beneath &
Use lower-case keywords and intrinsics (do, end subroutine, etc.)
Name modules with m_ pattern (e.g., m_transport)
Name public subroutines with s_ pattern (e.g., s_compute_flux)
Name public functions with f_ pattern
Keep subroutine size ≤ 500 lines, helper subroutines ≤ 150 lines, functions ≤ 100 lines, files ≤ 1000 lines
Limit routine arguments to ≤ 6; use derived-type params struct if more are needed
Forbid goto statements (except in legacy code), COMMON blocks, and save globals
Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Call s_mpi_abort() for errors, never use stop or error stop
**/*.{fpp,f90}: Indent 2 spaces; continuation lines align under&
Use lower-case keywords and intrinsics (do,end subroutine, etc.)
Name modules withm_<feature>prefix (e.g.,m_transport)
Name public subroutines ass_<verb>_<noun>(e.g.,s_compute_flux) and functions asf_<verb>_<noun>
Keep private helpers in the module; avoid nested procedures
Enforce size limits: subroutine ≤ 500 lines, helper ≤ 150, function ≤ 100, module/file ≤ 1000
Limit subroutines to ≤ 6 arguments; otherwise pass a derived-type 'params' struct
Avoidgotostatements (except unavoidable legacy); avoid global state (COMMON,save)
Every variable must haveintent(in|out|inout)specification and appropriatedimension/allocatable/pointer
Uses_mpi_abort(<msg>)for error termination instead ofstop
Use!>style documentation for header comments; follow Doxygen Fortran format with!! @paramand!! @returntags
Useimplicit nonestatement in all modules
Useprivatedeclaration followed by explicitpublicexports in modules
Use derived types with pointers for encapsulation (e.g.,pointer, dimension(:,:,:) => null())
Usepureandelementalattributes for side-effect-free functions; combine them for array ...
Files:
src/simulation/m_mpi_proxy.fppsrc/simulation/m_time_steppers.fppsrc/pre_process/m_global_parameters.fppsrc/common/m_mpi_common.fppsrc/common/m_derived_types.fppsrc/simulation/m_ibm.fpp
src/simulation/**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/simulation/**/*.{fpp,f90}: Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Avoid stop/error stop inside GPU device code
Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
src/simulation/**/*.{fpp,f90}: Mark GPU-callable helpers with$:GPU_ROUTINE(function_name='...', parallelism='[seq]')immediately after declaration
Do not use OpenACC or OpenMP directives directly; use Fypp macros fromsrc/common/include/parallel_macros.fppinstead
Wrap tight loops with$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')macro; addcollapse=nfor safe nested loop merging
Declare loop-local variables withprivate='[...]'in GPU parallel loop macros
Allocate large arrays withmanagedor move them into a persistent$:GPU_ENTER_DATA(...)region at start-up
Do not placestoporerror stopinside device code
Files:
src/simulation/m_mpi_proxy.fppsrc/simulation/m_time_steppers.fppsrc/simulation/m_ibm.fpp
src/**/*.fpp
📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)
src/**/*.fpp: Use.fppfile extension for Fypp preprocessed files; CMake transpiles them to.f90
Start module files with Fypp include for macros:#:include 'macros.fpp'
Use the fyppASSERTmacro for validating conditions:@:ASSERT(predicate, message)
Use fypp macro@:ALLOCATE(var1, var2)for device-aware allocation instead of standard Fortranallocate
Use fypp macro@:DEALLOCATE(var1, var2)for device-aware deallocation instead of standard Fortrandeallocate
Files:
src/simulation/m_mpi_proxy.fppsrc/simulation/m_time_steppers.fppsrc/pre_process/m_global_parameters.fppsrc/common/m_mpi_common.fppsrc/common/m_derived_types.fppsrc/simulation/m_ibm.fpp
🧠 Learnings (13)
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Add and update focused tests for changes; ensure no regressions in golden test outputs without clear justification
Applied to files:
tests/127A967A/golden-metadata.txttests/127A967A/golden.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`
Applied to files:
tests/127A967A/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
Applied to files:
tests/127A967A/golden-metadata.txtsrc/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Compile with Cray `ftn` or NVIDIA `nvfortran` for GPU offloading; also build CPU-only with GNU `gfortran` and Intel `ifx`/`ifort` for portability
Applied to files:
tests/127A967A/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Applied to files:
src/common/m_mpi_common.fppsrc/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Declare loop-local variables with `private='[...]'` in GPU parallel loop macros
Applied to files:
src/common/m_mpi_common.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Use `private` declaration followed by explicit `public` exports in modules
Applied to files:
src/common/m_mpi_common.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Do not use OpenACC or OpenMP directives directly; use Fypp macros from `src/common/include/parallel_macros.fpp` instead
Applied to files:
src/common/m_mpi_common.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Use `pure` and `elemental` attributes for side-effect-free functions; combine them for array operations
Applied to files:
src/common/m_mpi_common.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight loops with `$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')` macro; add `collapse=n` for safe nested loop merging
Applied to files:
src/common/m_mpi_common.fppsrc/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Mark GPU-callable helpers with `$:GPU_ROUTINE(function_name='...', parallelism='[seq]')` immediately after declaration
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large arrays with `managed` or move them into a persistent `$:GPU_ENTER_DATA(...)` region at start-up
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Applied to files:
src/simulation/m_ibm.fpp
🧬 Code graph analysis (3)
toolchain/bootstrap/modules.sh (1)
toolchain/util.sh (2)
log(12-12)log_n(13-13)
examples/2D_mibm_cylinder_in_cross_flow/case.py (1)
toolchain/mfc/test/cases.py (5)
alter_ib(377-422)alter_2d(293-320)alter_mixlayer_perturb(685-716)alter_num_fluids(224-291)alter_grcbc(78-120)
src/simulation/m_ibm.fpp (1)
toolchain/mfc/test/cases.py (1)
alter_ib(377-422)
🪛 markdownlint-cli2 (0.18.1)
docs/documentation/case.md
385-385: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
386-386: Hard tabs
Column: 13
(MD010, no-hard-tabs)
387-387: Hard tabs
Column: 12
(MD010, no-hard-tabs)
394-394: Hard tabs
Column: 15
(MD010, no-hard-tabs)
395-395: Hard tabs
Column: 13
(MD010, no-hard-tabs)
396-396: Hard tabs
Column: 16
(MD010, no-hard-tabs)
397-397: Hard tabs
Column: 10
(MD010, no-hard-tabs)
408-408: Hard tabs
Column: 108
(MD010, no-hard-tabs)
409-409: Hard tabs
Column: 172
(MD010, no-hard-tabs)
410-410: Hard tabs
Column: 14
(MD010, no-hard-tabs)
411-411: Hard tabs
Column: 132
(MD010, no-hard-tabs)
411-411: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
412-412: Hard tabs
Column: 99
(MD010, no-hard-tabs)
413-413: Hard tabs
Column: 23
(MD010, no-hard-tabs)
580-580: Hard tabs
Column: 80
(MD010, no-hard-tabs)
581-581: Hard tabs
Column: 61
(MD010, no-hard-tabs)
582-582: Hard tabs
Column: 51
(MD010, no-hard-tabs)
585-585: Hard tabs
Column: 64
(MD010, no-hard-tabs)
586-586: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
587-587: Hard tabs
Column: 78
(MD010, no-hard-tabs)
588-588: Hard tabs
Column: 85
(MD010, no-hard-tabs)
589-589: Hard tabs
Column: 85
(MD010, no-hard-tabs)
590-590: Hard tabs
Column: 75
(MD010, no-hard-tabs)
591-591: Hard tabs
Column: 71
(MD010, no-hard-tabs)
592-592: Hard tabs
Column: 91
(MD010, no-hard-tabs)
593-593: Hard tabs
Column: 91
(MD010, no-hard-tabs)
594-594: Hard tabs
Column: 82
(MD010, no-hard-tabs)
596-596: Hard tabs
Column: 89
(MD010, no-hard-tabs)
597-597: Hard tabs
Column: 74
(MD010, no-hard-tabs)
598-598: Hard tabs
Column: 86
(MD010, no-hard-tabs)
605-605: Hard tabs
Column: 97
(MD010, no-hard-tabs)
606-606: Hard tabs
Column: 55
(MD010, no-hard-tabs)
607-607: Hard tabs
Column: 33
(MD010, no-hard-tabs)
607-607: Hard tabs
Column: 62
(MD010, no-hard-tabs)
783-783: Hard tabs
Column: 30
(MD010, no-hard-tabs)
783-783: Hard tabs
Column: 66
(MD010, no-hard-tabs)
784-784: Hard tabs
Column: 30
(MD010, no-hard-tabs)
785-785: Hard tabs
Column: 13
(MD010, no-hard-tabs)
785-785: Hard tabs
Column: 28
(MD010, no-hard-tabs)
786-786: Hard tabs
Column: 11
(MD010, no-hard-tabs)
786-786: Hard tabs
Column: 24
(MD010, no-hard-tabs)
787-787: Hard tabs
Column: 30
(MD010, no-hard-tabs)
788-788: Hard tabs
Column: 8
(MD010, no-hard-tabs)
788-788: Hard tabs
Column: 28
(MD010, no-hard-tabs)
789-789: Hard tabs
Column: 16
(MD010, no-hard-tabs)
789-789: Hard tabs
Column: 28
(MD010, no-hard-tabs)
789-789: Hard tabs
Column: 31
(MD010, no-hard-tabs)
790-790: Hard tabs
Column: 8
(MD010, no-hard-tabs)
790-790: Hard tabs
Column: 25
(MD010, no-hard-tabs)
791-791: Hard tabs
Column: 9
(MD010, no-hard-tabs)
791-791: Hard tabs
Column: 24
(MD010, no-hard-tabs)
792-792: Hard tabs
Column: 12
(MD010, no-hard-tabs)
792-792: Hard tabs
Column: 26
(MD010, no-hard-tabs)
793-793: Hard tabs
Column: 10
(MD010, no-hard-tabs)
793-793: Hard tabs
Column: 30
(MD010, no-hard-tabs)
793-793: Hard tabs
Column: 32
(MD010, no-hard-tabs)
794-794: Hard tabs
Column: 15
(MD010, no-hard-tabs)
794-794: Hard tabs
Column: 29
(MD010, no-hard-tabs)
794-794: Hard tabs
Column: 31
(MD010, no-hard-tabs)
795-795: Hard tabs
Column: 10
(MD010, no-hard-tabs)
795-795: Hard tabs
Column: 28
(MD010, no-hard-tabs)
795-795: Hard tabs
Column: 31
(MD010, no-hard-tabs)
796-796: Hard tabs
Column: 10
(MD010, no-hard-tabs)
796-796: Hard tabs
Column: 28
(MD010, no-hard-tabs)
796-796: Hard tabs
Column: 31
(MD010, no-hard-tabs)
797-797: Hard tabs
Column: 10
(MD010, no-hard-tabs)
797-797: Hard tabs
Column: 28
(MD010, no-hard-tabs)
797-797: Hard tabs
Column: 31
(MD010, no-hard-tabs)
1040-1040: Hard tabs
Column: 23
(MD010, no-hard-tabs)
1041-1041: Hard tabs
Column: 17
(MD010, no-hard-tabs)
1042-1042: Hard tabs
Column: 20
(MD010, no-hard-tabs)
1043-1043: Hard tabs
Column: 21
(MD010, no-hard-tabs)
1044-1044: Hard tabs
Column: 18
(MD010, no-hard-tabs)
1045-1045: Hard tabs
Column: 14
(MD010, no-hard-tabs)
1046-1046: Hard tabs
Column: 21
(MD010, no-hard-tabs)
1047-1047: Hard tabs
Column: 17
(MD010, no-hard-tabs)
1048-1048: Hard tabs
Column: 17
(MD010, no-hard-tabs)
1049-1049: Hard tabs
Column: 19
(MD010, no-hard-tabs)
1050-1050: Hard tabs
Column: 22
(MD010, no-hard-tabs)
1051-1051: Hard tabs
Column: 20
(MD010, no-hard-tabs)
1052-1052: Hard tabs
Column: 21
(MD010, no-hard-tabs)
🪛 Shellcheck (0.11.0)
toolchain/bootstrap/modules.sh
[warning] 48-48: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 48-48: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
toolchain/util.sh
[warning] 8-8: R appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 8-8: C appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 8-8: G appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 8-8: BR appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 9-9: Y appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 9-9: M appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 9-9: B appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 9-9: OR appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 9-9: W appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Code Cleanliness Check
- GitHub Check: Coverage Test on CodeCov
- GitHub Check: Github (ubuntu, mpi, no-debug, true)
- GitHub Check: Github (ubuntu, mpi, debug, true)
- GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, debug, false)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Build & Publish
🔇 Additional comments (11)
src/simulation/m_mpi_proxy.fpp (1)
205-207: LGTM!The addition of
massto the MPI broadcast list is consistent with the new field added toib_patch_parametersinm_derived_types.fpp. The broadcast follows the existing pattern for scalarreal(wp)fields.toolchain/mfc/run/case_dicts.py (2)
118-118: LGTM!The addition of
massto thepatch_ibattribute mappings is correctly typed asParamType.REALand follows the existing pattern for other IB parameters.
367-367: LGTM!Symmetric addition of
massto the SIMULATION dictionary maintains consistency with PRE_PROCESS.src/simulation/m_time_steppers.fpp (2)
626-641: Approve the two-way coupling integration with a note on dependencies.The RK integration for velocity and angular velocity with force/torque coupling looks correct. However, this code depends on
s_compute_moment_of_inertiawhich has critical bugs (see m_ibm.fpp review). Once those are fixed, this integration should work correctly.Note: The angular momentum conservation approach (multiply by old moment, add torque impulse, recompute moment, divide by new moment) is physically sound for objects whose moment of inertia changes with rotation axis.
614-614: Verify force computation uses correct primitive variable.The call to
s_compute_ib_forcespassesq_prim_vf(E_idx)%sf(0:m, 0:n, 0:p). Confirm thatE_idxcorrectly indexes the pressure field in primitive variables (not energy as in conservative variables).src/simulation/m_ibm.fpp (2)
1133-1133: Invalid GPU update syntax for derived type field.The
$:GPU_UPDATEdirective withpatch_ib(ib_marker)%momentmay not work correctly. The entirepatch_ibarray should be updated instead.-$:GPU_UPDATE(device='[patch_ib(ib_marker)%moment]') +$:GPU_UPDATE(device='[patch_ib]')⛔ Skipped due to learnings
Learnt from: CR Repo: MFlowCode/MFC PR: 0 File: .cursor/rules/mfc-agent-rules.mdc:0-0 Timestamp: 2025-11-24T21:50:46.909Z Learning: Applies to src/simulation/**/*.{fpp,f90} : Mark GPU-callable helpers with `$:GPU_ROUTINE(function_name='...', parallelism='[seq]')` immediately after declarationLearnt from: CR Repo: MFlowCode/MFC PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-24T21:50:16.713Z Learning: Applies to src/simulation/**/*.{fpp,f90} : Avoid stop/error stop inside GPU device code
263-270: Redundant loop and unclear multi-fluid handling.The loop
do q = 1, num_fluidsiterates butqis never used in the loop body. The pressure is simply overwrittennum_fluidstimes with the same value. The TODO comment also suggests this needs attention.-! set the pressure -do q = 1, num_fluids - if (patch_ib(patch_id)%moving_ibm == 0) then - q_prim_vf(E_idx)%sf(j, k, l) = pres_IP - else - ! TODO :: improve for two fluid - q_prim_vf(E_idx)%sf(j, k, l) = pres_IP/(1._wp - 2._wp*abs(levelset%sf(j, k, l, patch_id)*alpha_rho_IP(q)/pres_IP)*dot_product(patch_ib(patch_id)%force/patch_ib(patch_id)%mass, levelset_norm%sf(j, k, l, patch_id, :))) - end if -end do +! set the pressure +if (patch_ib(patch_id)%moving_ibm == 0) then + q_prim_vf(E_idx)%sf(j, k, l) = pres_IP +else + ! TODO :: improve for two fluid + q_prim_vf(E_idx)%sf(j, k, l) = pres_IP/(1._wp - 2._wp*abs(levelset%sf(j, k, l, patch_id)*alpha_rho_IP(1)/pres_IP)*dot_product(patch_ib(patch_id)%force/patch_ib(patch_id)%mass, levelset_norm%sf(j, k, l, patch_id, :))) +end ifLikely an incorrect or invalid review comment.
toolchain/util.sh (1)
4-10: ORANGE/OR color additions look goodThe new
ORANGEescape andORalias integrate cleanly with the existing color scheme and are used by the updated module prompt without affecting existing behavior.tests/127A967A/golden-metadata.txt (1)
1-159: Golden metadata is consistent and helpful for reproducibilityThe metadata cleanly captures invocation, build configurations, and hardware details for test
127A967A, which aligns with the project’s guidance to document golden outputs for regressions tracking.tests/127A967A/golden.txt (1)
1-11: Data-only golden baseline looks fine; rely on tests to validateThis file is pure reference data (conservative fields and
ib_markers) for test127A967A. There’s nothing structurally concerning; just ensure CI re-runs this case and that changes to these values are intentional given the new IBM two-way coupling.src/pre_process/m_global_parameters.fpp (1)
534-565: Good: new IBMmassandmomentfields are now defaultedInitializing
patch_ib(i)%massandpatch_ib(i)%momenttodflt_realalongside the otherpatch_ibfields ins_assign_default_values_to_user_inputsavoids uninitialized state when these aren’t set in a case file and keeps the pre-process side consistent with the extendedib_patch_parameterstype.
| if (p == 0) then | ||
| axis = [0, 1, 0] | ||
| else if (sqrt(sum(axis**2)) == 0) then | ||
| ! if the object is not actually rotating at this time, return a dummy value and exit | ||
| patch_ib(ib_marker)%moment = 1._wp | ||
| return | ||
| else | ||
| axis = axis/sqrt(sum(axis)) | ||
| end if |
There was a problem hiding this comment.
Optional argument handling issue in 2D case.
When p == 0 (2D), the code assigns to axis without checking if it was provided. Since axis is declared optional, assigning to it when not present is undefined behavior. Additionally, the axis parameter should have intent(inout) if it's being modified.
-subroutine s_compute_moment_of_inertia(ib_marker, axis)
+subroutine s_compute_moment_of_inertia(ib_marker, axis_in)
- real(wp), dimension(3), optional :: axis !< the axis about which we compute the moment. Only required in 3D.
+ real(wp), dimension(3), intent(in), optional :: axis_in !< the axis about which we compute the moment. Only required in 3D.
integer, intent(in) :: ib_marker
real(wp) :: moment, distance_to_axis, cell_volume
- real(wp), dimension(3) :: position, closest_point_along_axis, vector_to_axis
+ real(wp), dimension(3) :: position, closest_point_along_axis, vector_to_axis, axis
integer :: i, j, k, count
if (p == 0) then
- axis = [0, 1, 0]
- else if (sqrt(sum(axis**2)) == 0) then
+ axis = [0._wp, 0._wp, 1._wp]
+ else if (.not. present(axis_in) .or. sqrt(sum(axis_in**2)) < 1.e-16_wp) then
! if the object is not actually rotating at this time, return a dummy value and exit
patch_ib(ib_marker)%moment = 1._wp
return
else
- axis = axis/sqrt(sum(axis))
+ axis = axis_in/sqrt(sum(axis_in**2))
end ifCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/simulation/m_ibm.fpp around lines 1076 to 1084, the branch for p == 0
assigns to the optional argument axis (undefined if not present) and axis should
be declared intent(inout) if modified; fix by (a) guarding any write/read of
axis with present(axis) (e.g., only assign axis = [0,1,0] when present(axis) is
true), (b) avoid touching axis when not present (use a local dummy variable
instead), and (c) update the routine signature to declare axis as intent(inout)
if the caller is expected to receive the modified axis. Ensure other uses in
this block also check present(axis) before reading or normalizing it.
…eed this up now with reductions
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/simulation/m_ibm.fpp">
<violation number="1" location="src/simulation/m_ibm.fpp:1013">
P1: Loop variable `l` is missing from the `private` clause of the GPU_PARALLEL_LOOP. This can cause race conditions on GPU since multiple threads will share the same loop variable. Add `l` to the private list to match the pattern used elsewhere in this file (e.g., lines 213, 397).</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/simulation/m_ibm.fpp (3)
260-270: Guard modified-pressure formula against division by zero and fix comment typoThe moving-IB pressure update divides by both
pres_IPand a denominator that can approach zero:q_prim_vf(E_idx)%sf(j, k, l) = q_prim_vf(E_idx)%sf(j, k, l) + pres_IP / (1._wp - 2._wp*abs(levelset%sf(...)*alpha_rho_IP(q)/pres_IP)*dot_product(...))If
pres_IP == 0or the denominator is (near) zero, this will generate NaNs/inf and can destabilize the solution. There is also no protection againstpatch_ib(patch_id)%mass == 0._wp.This issue has been flagged in earlier reviews and is still present; adding robust guards here is important.
You can address both stability and the typo in the comment as follows:
- if (patch_ib(patch_id)%moving_ibm <= 1) then - q_prim_vf(E_idx)%sf(j, k, l) = pres_IP - else - q_prim_vf(E_idx)%sf(j, k, l) = 0._wp - $:GPU_LOOP(parallelism='[seq]') - do q = 1, num_fluids - ! Se the pressure inside a moving immersed boundary based upon the pressure of the image point. acceleration, and normal vector direction - q_prim_vf(E_idx)%sf(j, k, l) = q_prim_vf(E_idx)%sf(j, k, l) + pres_IP/(1._wp - 2._wp*abs(levelset%sf(j, k, l, patch_id)*alpha_rho_IP(q)/pres_IP)*dot_product(patch_ib(patch_id)%force/patch_ib(patch_id)%mass, levelset_norm%sf(j, k, l, patch_id, :))) - end do - end if + if (patch_ib(patch_id)%moving_ibm <= 1) then + q_prim_vf(E_idx)%sf(j, k, l) = pres_IP + else + q_prim_vf(E_idx)%sf(j, k, l) = 0._wp + if (abs(pres_IP) > 1.e-16_wp .and. patch_ib(patch_id)%mass > 0._wp) then + $:GPU_LOOP(parallelism='[seq]') + do q = 1, num_fluids + ! Set the pressure inside a moving immersed boundary based upon the pressure of the image point, + ! acceleration, and normal vector direction + buf = 1._wp - 2._wp*abs(levelset%sf(j, k, l, patch_id)*alpha_rho_IP(q)/pres_IP) * & + dot_product(patch_ib(patch_id)%force/patch_ib(patch_id)%mass, levelset_norm%sf(j, k, l, patch_id, :)) + if (abs(buf) > 1.e-16_wp) then + q_prim_vf(E_idx)%sf(j, k, l) = q_prim_vf(E_idx)%sf(j, k, l) + pres_IP/buf + else + q_prim_vf(E_idx)%sf(j, k, l) = q_prim_vf(E_idx)%sf(j, k, l) + pres_IP + end if + end do + else + q_prim_vf(E_idx)%sf(j, k, l) = pres_IP + end if + end if
1063-1068: Remove or gate debug print of forcesThe unconditional
print *, forces(1, 1:2)will spam stdout (across ranks) and hurt performance for production runs.
Please remove it or guard it behind an existing debug/verbosity flag.
- print *, forces(1, 1:2)
1081-1150: Fix optionalaxishandling and normalization ins_compute_moment_of_inertia
axisis declaredoptionalbut is used and assigned unconditionally:real(wp), dimension(3), optional :: axis ... if (p == 0) then axis = [0, 1, 0] else if (sqrt(sum(axis**2)) == 0) then ... else axis = axis/sqrt(sum(axis)) end ifUsing or assigning an absent optional argument is undefined behavior. Additionally, the normalization uses
sqrt(sum(axis))instead ofsqrt(sum(axis**2)), which is mathematically incorrect. This has been noted in prior automated reviews and remains in the current code.A safe restructuring is to treat the optional argument as an
intent(in)input, copy it into a localaxisvector, and always work with the local copy:- subroutine s_compute_moment_of_inertia(ib_marker, axis) - - real(wp), dimension(3), optional :: axis !< the axis about which we compute the moment. Only required in 3D. - integer, intent(in) :: ib_marker - - real(wp) :: moment, distance_to_axis, cell_volume - real(wp), dimension(3) :: position, closest_point_along_axis, vector_to_axis + subroutine s_compute_moment_of_inertia(ib_marker, axis_in) + + integer, intent(in) :: ib_marker + real(wp), dimension(3), intent(in), optional :: axis_in !< Axis about which we compute the moment (required in 3D). + + real(wp) :: moment, distance_to_axis, cell_volume + real(wp), dimension(3) :: position, closest_point_along_axis, vector_to_axis, axis integer :: i, j, k, count - - if (p == 0) then - axis = [0, 1, 0] - else if (sqrt(sum(axis**2)) == 0) then - ! if the object is not actually rotating at this time, return a dummy value and exit - patch_ib(ib_marker)%moment = 1._wp - return - else - axis = axis/sqrt(sum(axis)) - end if + ! + ! Define/normalize rotation axis. + ! + if (p == 0) then + ! 2D: rotation axis is out of plane (z-direction). + axis = [0._wp, 0._wp, 1._wp] + else + if (.not. present(axis_in)) then + call s_mpi_abort("s_compute_moment_of_inertia: axis must be provided in 3D") + else if (sqrt(sum(axis_in**2)) <= 1.e-16_wp) then + ! If the object is not actually rotating at this time, return a dummy value and exit + patch_ib(ib_marker)%moment = 1._wp + return + else + axis = axis_in/sqrt(sum(axis_in**2)) + end if + end if @@ - $:GPU_PARALLEL_LOOP(private='[position,closest_point_along_axis,vector_to_axis,distance_to_axis]', copy='[moment,count]', copyin='[ib_marker,cell_volume,axis]', collapse=3) + $:GPU_PARALLEL_LOOP(private='[position,closest_point_along_axis,vector_to_axis,distance_to_axis]', copy='[moment,count]', copyin='[ib_marker,cell_volume,axis]', collapse=3)This (1) avoids undefined behavior with absent optionals, (2) fixes the normalization to use
sum(axis_in**2), and (3) makes the requiredintentexplicit, as per the coding guidelines.
🧹 Nitpick comments (1)
src/simulation/m_ibm.fpp (1)
199-211: Restrict interior-pressure initialization to moving IBs and fix GPU privatesThis loop currently sets
q_prim_vf(E_idx)%sfto1._wpfor all cells withib_markers%sf /= 0, including fixed IBs. That differs from the stated intent (initialize interiors only for moving IBs) and leaves inner cells of stationary IBs with a hard-coded pressure of 1.0 that is never overwritten.Also,
lis a loop index but is not listed in theprivateclause, which goes against the GPU macro guidelines (even if most backends make it private implicitly).You can make the intent explicit and restrict to truly moving IBs with something like:
- ! set the Moving IBM interior Pressure Values - $:GPU_PARALLEL_LOOP(private='[i,j,k]', copyin='[E_idx]', collapse=3) - do l = 0, p - do k = 0, n - do j = 0, m - if (ib_markers%sf(j, k, l) /= 0) then - q_prim_vf(E_idx)%sf(j, k, l) = 1._wp - end if - end do - end do - end do + ! Set interior pressure values for moving IBs + $:GPU_PARALLEL_LOOP(private='[j,k,l,patch_id]', copyin='[E_idx]', collapse=3) + do l = 0, p + do k = 0, n + do j = 0, m + patch_id = ib_markers%sf(j, k, l) + if (patch_id /= 0 .and. patch_ib(patch_id)%moving_ibm > 1) then + q_prim_vf(E_idx)%sf(j, k, l) = 1._wp + end if + end do + end do + end do $:END_GPU_PARALLEL_LOOP()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/simulation/m_ibm.fpp(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{fpp,f90}: Use 2-space indentation; continuation lines align beneath &
Use lower-case keywords and intrinsics (do, end subroutine, etc.)
Name modules with m_ pattern (e.g., m_transport)
Name public subroutines with s_ pattern (e.g., s_compute_flux)
Name public functions with f_ pattern
Keep subroutine size ≤ 500 lines, helper subroutines ≤ 150 lines, functions ≤ 100 lines, files ≤ 1000 lines
Limit routine arguments to ≤ 6; use derived-type params struct if more are needed
Forbid goto statements (except in legacy code), COMMON blocks, and save globals
Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Call s_mpi_abort() for errors, never use stop or error stop
**/*.{fpp,f90}: Indent 2 spaces; continuation lines align under&
Use lower-case keywords and intrinsics (do,end subroutine, etc.)
Name modules withm_<feature>prefix (e.g.,m_transport)
Name public subroutines ass_<verb>_<noun>(e.g.,s_compute_flux) and functions asf_<verb>_<noun>
Keep private helpers in the module; avoid nested procedures
Enforce size limits: subroutine ≤ 500 lines, helper ≤ 150, function ≤ 100, module/file ≤ 1000
Limit subroutines to ≤ 6 arguments; otherwise pass a derived-type 'params' struct
Avoidgotostatements (except unavoidable legacy); avoid global state (COMMON,save)
Every variable must haveintent(in|out|inout)specification and appropriatedimension/allocatable/pointer
Uses_mpi_abort(<msg>)for error termination instead ofstop
Use!>style documentation for header comments; follow Doxygen Fortran format with!! @paramand!! @returntags
Useimplicit nonestatement in all modules
Useprivatedeclaration followed by explicitpublicexports in modules
Use derived types with pointers for encapsulation (e.g.,pointer, dimension(:,:,:) => null())
Usepureandelementalattributes for side-effect-free functions; combine them for array ...
Files:
src/simulation/m_ibm.fpp
src/simulation/**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/simulation/**/*.{fpp,f90}: Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Avoid stop/error stop inside GPU device code
Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
src/simulation/**/*.{fpp,f90}: Mark GPU-callable helpers with$:GPU_ROUTINE(function_name='...', parallelism='[seq]')immediately after declaration
Do not use OpenACC or OpenMP directives directly; use Fypp macros fromsrc/common/include/parallel_macros.fppinstead
Wrap tight loops with$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')macro; addcollapse=nfor safe nested loop merging
Declare loop-local variables withprivate='[...]'in GPU parallel loop macros
Allocate large arrays withmanagedor move them into a persistent$:GPU_ENTER_DATA(...)region at start-up
Do not placestoporerror stopinside device code
Files:
src/simulation/m_ibm.fpp
src/**/*.fpp
📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)
src/**/*.fpp: Use.fppfile extension for Fypp preprocessed files; CMake transpiles them to.f90
Start module files with Fypp include for macros:#:include 'macros.fpp'
Use the fyppASSERTmacro for validating conditions:@:ASSERT(predicate, message)
Use fypp macro@:ALLOCATE(var1, var2)for device-aware allocation instead of standard Fortranallocate
Use fypp macro@:DEALLOCATE(var1, var2)for device-aware deallocation instead of standard Fortrandeallocate
Files:
src/simulation/m_ibm.fpp
🧠 Learnings (14)
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight loops with `$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')` macro; add `collapse=n` for safe nested loop merging
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large arrays with `managed` or move them into a persistent `$:GPU_ENTER_DATA(...)` region at start-up
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Mark GPU-callable helpers with `$:GPU_ROUTINE(function_name='...', parallelism='[seq]')` immediately after declaration
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Avoid stop/error stop inside GPU device code
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to **/*.{fpp,f90} : Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Every variable must have `intent(in|out|inout)` specification and appropriate `dimension` / `allocatable` / `pointer`
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Declare loop-local variables with `private='[...]'` in GPU parallel loop macros
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Favor array operations over explicit loops when possible; use `collapse=N` directive to optimize nested loops
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Avoid `goto` statements (except unavoidable legacy); avoid global state (`COMMON`, `save`)
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Each variable should have one clear purpose; do not use the same variable for multiple purposes
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Do not use OpenACC or OpenMP directives directly; use Fypp macros from `src/common/include/parallel_macros.fpp` instead
Applied to files:
src/simulation/m_ibm.fpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Coverage Test on CodeCov
- GitHub Check: Code Cleanliness Check
- GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, no-debug, false)
- GitHub Check: Github (macos, mpi, debug, false)
- GitHub Check: Github (ubuntu, mpi, debug, false)
- GitHub Check: Github (ubuntu, mpi, no-debug, true)
- GitHub Check: Github (macos, mpi, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, debug, true)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Build & Publish
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
src/simulation/m_time_steppers.fpp (2)
637-640: Questionable physics: Recomputing moment of inertia within RK sub-steps.The moment of inertia tensor for a rigid body in the body-fixed frame is constant. Recomputing it at each RK sub-step based on angular momentum direction could introduce numerical inconsistencies. Typically, the inertia tensor is fixed and only transformed to the lab frame when needed.
Please verify that this approach is physically correct for your use case. If the intent is to handle variable axis of rotation, consider using the full inertia tensor rather than a scalar moment that changes with direction.
630-641: Redundants_compute_ib_forcescall inside the IB loop.The call to
s_compute_ib_forcesat line 632 is redundant since it's already called once at line 614 before the loop.s_compute_ib_forcescomputes forces for ALL IBs in a single call (including MPI reductions). Calling it again insidedo i = 1, num_ibswill recompute forces multiple times unnecessarily, which is expensive due to MPI all-reduce operations.Remove the redundant call inside the loop:
if (patch_ib(i)%moving_ibm == 2) then ! if we are using two-way coupling, apply force and torque - ! compute the force and torque on the IB from the fluid - call s_compute_ib_forces(q_prim_vf(E_idx)%sf(0:m, 0:n, 0:p)) - ! update the velocity from the force value patch_ib(i)%vel = patch_ib(i)%vel + rk_coef(s, 3)*dt*(patch_ib(i)%force/patch_ib(i)%mass)/rk_coef(s, 4)src/simulation/m_ibm.fpp (3)
1015-1058: Missing loop variable in private clause and out-of-bounds access at domain boundaries.Two issues:
Missing
lin private clause: The inner loopdo l = 1, 3at line 1047 useslas an iterator, butlis not listed in the private clause. This can cause race conditions on GPU.Out-of-bounds pressure access: At boundary cells (i=0, i=m, j=0, j=n, k=0, k=p), the central difference stencil accesses
pressure(i±1, j, k)etc., which goes out of bounds. The pressure array is declareddimension(0:m, 0:n, 0:p).- $:GPU_PARALLEL_LOOP(private='[ib_idx,radial_vector,pressure_divergence,cell_volume,temp_torque_vector, dx, dy, dz]', copy='[forces,torques]', copyin='[ib_markers]', collapse=3) - do i = 0, m - do j = 0, n - do k = 0, p + $:GPU_PARALLEL_LOOP(private='[ib_idx,radial_vector,pressure_divergence,cell_volume,temp_torque_vector,dx,dy,dz,l]', copy='[forces,torques]', copyin='[ib_markers]', collapse=3) + do i = 1, m - 1 + do j = 1, n - 1 + do k = 0, p ib_idx = ib_markers%sf(i, j, k) - if (ib_idx /= 0) then + if (ib_idx /= 0 .and. (p == 0 .or. (k > 0 .and. k < p))) then
1090-1098: Incorrect axis normalization and unsafe optional argument handling.Two issues:
Wrong normalization formula: Line 1097 uses
sqrt(sum(axis))but should usesqrt(sum(axis**2))to compute the vector magnitude. The sum of components is not the magnitude.Unsafe optional argument: The code assigns to
axis(line 1091, 1097) without checkingpresent(axis). In 2D whenp == 0, assigning to an optional argument that wasn't passed is undefined behavior.- if (p == 0) then - axis = [0, 1, 0] - else if (sqrt(sum(axis**2)) == 0) then + real(wp), dimension(3) :: local_axis + + if (p == 0) then + local_axis = [0._wp, 0._wp, 1._wp] + else if (.not. present(axis) .or. sqrt(sum(axis**2)) < 1.e-16_wp) then ! if the object is not actually rotating at this time, return a dummy value and exit patch_ib(ib_marker)%moment = 1._wp return else - axis = axis/sqrt(sum(axis)) + local_axis = axis/sqrt(sum(axis**2)) end ifThen use
local_axisthroughout the subroutine instead ofaxis.
259-270: Incorrect pressure accumulation and division-by-zero risk.Several issues in this block:
Incorrect loop logic: The loop over
num_fluidsaddspres_IP/denominatoreach iteration, effectively multiplying the pressure bynum_fluids. The TODO comment acknowledges this needs improvement.Division-by-zero risk: If
pres_IPis zero or near-zero, or if the denominator(1 - 2*|...|)approaches zero, this will produce NaN/Inf.Typo: Line 267 has "Se the pressure" instead of "Set the pressure".
Consider guarding against division by zero and fixing the multi-fluid logic:
else q_prim_vf(E_idx)%sf(j, k, l) = 0._wp $:GPU_LOOP(parallelism='[seq]') do q = 1, num_fluids - ! Se the pressure inside a moving immersed boundary based upon the pressure of the image point. acceleration, and normal vector direction - q_prim_vf(E_idx)%sf(j, k, l) = q_prim_vf(E_idx)%sf(j, k, l) + pres_IP/(1._wp - 2._wp*abs(levelset%sf(j, k, l, patch_id)*alpha_rho_IP(q)/pres_IP)*dot_product(patch_ib(patch_id)%force/patch_ib(patch_id)%mass, levelset_norm%sf(j, k, l, patch_id, :))) + ! Set the pressure inside a moving immersed boundary based upon the pressure of the image point, acceleration, and normal vector direction + ! TODO: Proper multi-fluid weighting needed + if (abs(pres_IP) > 1.e-16_wp) then + buf = 1._wp - 2._wp*abs(levelset%sf(j, k, l, patch_id)*alpha_rho_IP(q)/pres_IP)*dot_product(patch_ib(patch_id)%force/patch_ib(patch_id)%mass, levelset_norm%sf(j, k, l, patch_id, :)) + if (abs(buf) > 1.e-16_wp) then + q_prim_vf(E_idx)%sf(j, k, l) = q_prim_vf(E_idx)%sf(j, k, l) + pres_IP/buf + end if + end if end do end if
🧹 Nitpick comments (1)
src/simulation/m_ibm.fpp (1)
1100-1107: Use working precision suffix for literal constants.Per coding guidelines, use
_wpsuffix for real constants to ensure consistent precision:if (patch_ib(ib_marker)%geometry == 2) then ! circle - patch_ib(ib_marker)%moment = 0.5*patch_ib(ib_marker)%mass*(patch_ib(ib_marker)%radius)**2 + patch_ib(ib_marker)%moment = 0.5_wp*patch_ib(ib_marker)%mass*(patch_ib(ib_marker)%radius)**2 elseif (patch_ib(ib_marker)%geometry == 3) then ! rectangle patch_ib(ib_marker)%moment = patch_ib(ib_marker)%mass*(patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2)/6._wp elseif (patch_ib(ib_marker)%geometry == 8) then ! sphere - patch_ib(ib_marker)%moment = 0.4*patch_ib(ib_marker)%mass*(patch_ib(ib_marker)%radius)**2 + patch_ib(ib_marker)%moment = 0.4_wp*patch_ib(ib_marker)%mass*(patch_ib(ib_marker)%radius)**2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/simulation/m_ibm.fpp(4 hunks)src/simulation/m_time_steppers.fpp(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{fpp,f90}: Use 2-space indentation; continuation lines align beneath &
Use lower-case keywords and intrinsics (do, end subroutine, etc.)
Name modules with m_ pattern (e.g., m_transport)
Name public subroutines with s_ pattern (e.g., s_compute_flux)
Name public functions with f_ pattern
Keep subroutine size ≤ 500 lines, helper subroutines ≤ 150 lines, functions ≤ 100 lines, files ≤ 1000 lines
Limit routine arguments to ≤ 6; use derived-type params struct if more are needed
Forbid goto statements (except in legacy code), COMMON blocks, and save globals
Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Call s_mpi_abort() for errors, never use stop or error stop
**/*.{fpp,f90}: Indent 2 spaces; continuation lines align under&
Use lower-case keywords and intrinsics (do,end subroutine, etc.)
Name modules withm_<feature>prefix (e.g.,m_transport)
Name public subroutines ass_<verb>_<noun>(e.g.,s_compute_flux) and functions asf_<verb>_<noun>
Keep private helpers in the module; avoid nested procedures
Enforce size limits: subroutine ≤ 500 lines, helper ≤ 150, function ≤ 100, module/file ≤ 1000
Limit subroutines to ≤ 6 arguments; otherwise pass a derived-type 'params' struct
Avoidgotostatements (except unavoidable legacy); avoid global state (COMMON,save)
Every variable must haveintent(in|out|inout)specification and appropriatedimension/allocatable/pointer
Uses_mpi_abort(<msg>)for error termination instead ofstop
Use!>style documentation for header comments; follow Doxygen Fortran format with!! @paramand!! @returntags
Useimplicit nonestatement in all modules
Useprivatedeclaration followed by explicitpublicexports in modules
Use derived types with pointers for encapsulation (e.g.,pointer, dimension(:,:,:) => null())
Usepureandelementalattributes for side-effect-free functions; combine them for array ...
Files:
src/simulation/m_ibm.fppsrc/simulation/m_time_steppers.fpp
src/simulation/**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/simulation/**/*.{fpp,f90}: Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Avoid stop/error stop inside GPU device code
Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
src/simulation/**/*.{fpp,f90}: Mark GPU-callable helpers with$:GPU_ROUTINE(function_name='...', parallelism='[seq]')immediately after declaration
Do not use OpenACC or OpenMP directives directly; use Fypp macros fromsrc/common/include/parallel_macros.fppinstead
Wrap tight loops with$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')macro; addcollapse=nfor safe nested loop merging
Declare loop-local variables withprivate='[...]'in GPU parallel loop macros
Allocate large arrays withmanagedor move them into a persistent$:GPU_ENTER_DATA(...)region at start-up
Do not placestoporerror stopinside device code
Files:
src/simulation/m_ibm.fppsrc/simulation/m_time_steppers.fpp
src/**/*.fpp
📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)
src/**/*.fpp: Use.fppfile extension for Fypp preprocessed files; CMake transpiles them to.f90
Start module files with Fypp include for macros:#:include 'macros.fpp'
Use the fyppASSERTmacro for validating conditions:@:ASSERT(predicate, message)
Use fypp macro@:ALLOCATE(var1, var2)for device-aware allocation instead of standard Fortranallocate
Use fypp macro@:DEALLOCATE(var1, var2)for device-aware deallocation instead of standard Fortrandeallocate
Files:
src/simulation/m_ibm.fppsrc/simulation/m_time_steppers.fpp
🧠 Learnings (14)
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight loops with `$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')` macro; add `collapse=n` for safe nested loop merging
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
Applied to files:
src/simulation/m_ibm.fppsrc/simulation/m_time_steppers.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large arrays with `managed` or move them into a persistent `$:GPU_ENTER_DATA(...)` region at start-up
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Mark GPU-callable helpers with `$:GPU_ROUTINE(function_name='...', parallelism='[seq]')` immediately after declaration
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Declare loop-local variables with `private='[...]'` in GPU parallel loop macros
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Do not use OpenACC or OpenMP directives directly; use Fypp macros from `src/common/include/parallel_macros.fpp` instead
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Avoid stop/error stop inside GPU device code
Applied to files:
src/simulation/m_ibm.fppsrc/simulation/m_time_steppers.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to **/*.{fpp,f90} : Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Every variable must have `intent(in|out|inout)` specification and appropriate `dimension` / `allocatable` / `pointer`
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Favor array operations over explicit loops when possible; use `collapse=N` directive to optimize nested loops
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Avoid `goto` statements (except unavoidable legacy); avoid global state (`COMMON`, `save`)
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Each variable should have one clear purpose; do not use the same variable for multiple purposes
Applied to files:
src/simulation/m_ibm.fpp
🪛 GitHub Actions: Pretty
src/simulation/m_ibm.fpp
[error] 1000-1000: MFC formatting changed argument alignment in subroutine s_compute_ib_forces; formatting step failed with exit code 1.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: Coverage Test on CodeCov
- GitHub Check: Code Cleanliness Check
- GitHub Check: Github (macos, mpi, debug, false)
- GitHub Check: Github (ubuntu, mpi, debug, true)
- GitHub Check: Self Hosted (gpu, omp, gt)
- GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, no-debug, true)
- GitHub Check: Github (macos, mpi, no-debug, false)
- GitHub Check: Self Hosted (cpu, none, gt)
- GitHub Check: Github (ubuntu, mpi, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, debug, false)
- GitHub Check: Self Hosted (cpu, none, frontier)
- GitHub Check: Self Hosted (gpu, omp, frontier)
- GitHub Check: Self Hosted (gpu, acc, gt)
- GitHub Check: Self Hosted (gpu, acc, frontier)
- GitHub Check: Build & Publish
🔇 Additional comments (1)
src/simulation/m_time_steppers.fpp (1)
614-615: Force computation moved outside the loop - good improvement.The
s_compute_ib_forcescall is now correctly placed before the IB loop, computing forces for all IBs in a single pass with one MPI reduction. However, note thatq_prim_vf(E_idx)%sfis passed without explicit bounds here (line 614) while line 632 uses explicit bounds(0:m, 0:n, 0:p)- ensure consistency.
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
src/simulation/m_ibm.fpp (5)
267-267: Fix typo in comment."Se the pressure" should be "Set the pressure".
Apply this diff:
- ! Se the pressure inside a moving immersed boundary based upon the pressure of the image point. acceleration, and normal vector direction + ! Set the pressure inside a moving immersed boundary based upon the pressure of the image point, acceleration, and normal vector direction
1092-1100: Fix optional argument handling and axis normalization.Two issues:
Optional argument violation: Line 1093 assigns to
axiswithout checking if it was provided viapresent(axis). Writing to an absent optional argument is undefined behavior in Fortran.Incorrect normalization: Line 1099 uses
sum(axis)(sum of components) instead ofsqrt(sum(axis**2))(magnitude). This will produce incorrect normalization.Apply this diff:
+ real(wp), dimension(3) :: axis_local + if (p == 0) then - axis = [0, 1, 0] - else if (sqrt(sum(axis**2)) == 0) then + axis_local = [0._wp, 0._wp, 1._wp] + else if (.not. present(axis)) then + ! If axis not provided in 3D, return dummy value + patch_ib(ib_marker)%moment = 1._wp + return + else if (sqrt(sum(axis**2)) < 1.e-16_wp) then ! if the object is not actually rotating at this time, return a dummy value and exit patch_ib(ib_marker)%moment = 1._wp return else - axis = axis/sqrt(sum(axis)) + axis_local = axis/sqrt(sum(axis**2)) end ifThen replace all uses of
axisin the routine withaxis_local.
199-210: Fix loop iterator mismatch in private clause and restrict pressure initialization to two-way coupled IBs.Two issues:
Private clause mismatch: The loop iterators are
l,k,j, but the private clause lists[i,j,k]. This should be[l,k,j]to match the actual loop variables.Overly broad initialization: This sets pressure to
1.0for ALL cells inside ANY IB (ib_markers%sf(j,k,l) /= 0), including stationary IBs. Based on the PR context (two-way coupling), this should only apply to IBs withmoving_ibm == 2.Apply this diff:
- $:GPU_PARALLEL_LOOP(private='[i,j,k]', copyin='[E_idx]', collapse=3) + $:GPU_PARALLEL_LOOP(private='[l,k,j,patch_id]', copyin='[E_idx,patch_ib]', collapse=3) do l = 0, p do k = 0, n do j = 0, m - if (ib_markers%sf(j, k, l) /= 0) then + patch_id = ib_markers%sf(j, k, l) + if (patch_id /= 0 .and. patch_ib(patch_id)%moving_ibm == 2) then q_prim_vf(E_idx)%sf(j, k, l) = 1._wp end if end doAs per coding guidelines for GPU parallelism.
1016-1016: Add loop variablelto private clause.The inner loop at lines 1049-1054 uses
las an iterator, but it is not listed in the private clause. This can cause race conditions on GPU since multiple threads will share the same loop variable.Apply this diff:
- $:GPU_PARALLEL_LOOP(private='[ib_idx,radial_vector,pressure_divergence,cell_volume,local_torque_contribution, dx, dy, dz]', copy='[forces,torques]', copyin='[ib_markers,patch_ib]', collapse=3) + $:GPU_PARALLEL_LOOP(private='[ib_idx,radial_vector,pressure_divergence,cell_volume,local_torque_contribution,dx,dy,dz,l]', copy='[forces,torques]', copyin='[ib_markers,patch_ib]', collapse=3)As per coding guidelines for GPU loop variables.
264-270: Redundant loop overwrites pressure value and division-by-zero risk.Two critical issues:
Redundant loop: The loop
do q = 1, num_fluidsrepeatedly overwritesq_prim_vf(E_idx)%sf(j, k, l)with each iteration—only the final fluid's contribution is retained. This is likely a logic error. Either:
- Remove the loop and compute once using a representative fluid index, or
- Accumulate contributions (initialize to zero, sum inside loop)
Division-by-zero risk: The formula divides by
pres_IPand a computed denominator with no guard. Ifpres_IP == 0or the denominator becomes zero/very small, this produces NaNs or runtime errors.Apply this diff to remove the loop and add zero guards:
! set the pressure if (patch_ib(patch_id)%moving_ibm <= 1) then q_prim_vf(E_idx)%sf(j, k, l) = pres_IP else - q_prim_vf(E_idx)%sf(j, k, l) = 0._wp - $:GPU_LOOP(parallelism='[seq]') - do q = 1, num_fluids - ! Se the pressure inside a moving immersed boundary based upon the pressure of the image point. acceleration, and normal vector direction - q_prim_vf(E_idx)%sf(j, k, l) = q_prim_vf(E_idx)%sf(j, k, l) + pres_IP/(1._wp - 2._wp*abs(levelset%sf(j, k, l, patch_id)*alpha_rho_IP(q)/pres_IP)*dot_product(patch_ib(patch_id)%force/patch_ib(patch_id)%mass, levelset_norm%sf(j, k, l, patch_id, :))) - end do + ! TODO: improve for multi-fluid - currently uses first fluid's properties + if (abs(pres_IP) > 1.e-16_wp) then + buf = 1._wp - 2._wp*abs(levelset%sf(j, k, l, patch_id)*alpha_rho_IP(1)/pres_IP)* & + dot_product(patch_ib(patch_id)%force/patch_ib(patch_id)%mass, levelset_norm%sf(j, k, l, patch_id, :)) + if (abs(buf) > 1.e-16_wp) then + q_prim_vf(E_idx)%sf(j, k, l) = pres_IP/buf + else + q_prim_vf(E_idx)%sf(j, k, l) = pres_IP + end if + else + q_prim_vf(E_idx)%sf(j, k, l) = pres_IP + end if end if
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/simulation/m_ibm.fpp(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{fpp,f90}: Use 2-space indentation; continuation lines align beneath &
Use lower-case keywords and intrinsics (do, end subroutine, etc.)
Name modules with m_ pattern (e.g., m_transport)
Name public subroutines with s_ pattern (e.g., s_compute_flux)
Name public functions with f_ pattern
Keep subroutine size ≤ 500 lines, helper subroutines ≤ 150 lines, functions ≤ 100 lines, files ≤ 1000 lines
Limit routine arguments to ≤ 6; use derived-type params struct if more are needed
Forbid goto statements (except in legacy code), COMMON blocks, and save globals
Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Call s_mpi_abort() for errors, never use stop or error stop
**/*.{fpp,f90}: Indent 2 spaces; continuation lines align under&
Use lower-case keywords and intrinsics (do,end subroutine, etc.)
Name modules withm_<feature>prefix (e.g.,m_transport)
Name public subroutines ass_<verb>_<noun>(e.g.,s_compute_flux) and functions asf_<verb>_<noun>
Keep private helpers in the module; avoid nested procedures
Enforce size limits: subroutine ≤ 500 lines, helper ≤ 150, function ≤ 100, module/file ≤ 1000
Limit subroutines to ≤ 6 arguments; otherwise pass a derived-type 'params' struct
Avoidgotostatements (except unavoidable legacy); avoid global state (COMMON,save)
Every variable must haveintent(in|out|inout)specification and appropriatedimension/allocatable/pointer
Uses_mpi_abort(<msg>)for error termination instead ofstop
Use!>style documentation for header comments; follow Doxygen Fortran format with!! @paramand!! @returntags
Useimplicit nonestatement in all modules
Useprivatedeclaration followed by explicitpublicexports in modules
Use derived types with pointers for encapsulation (e.g.,pointer, dimension(:,:,:) => null())
Usepureandelementalattributes for side-effect-free functions; combine them for array ...
Files:
src/simulation/m_ibm.fpp
src/simulation/**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/simulation/**/*.{fpp,f90}: Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Avoid stop/error stop inside GPU device code
Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
src/simulation/**/*.{fpp,f90}: Mark GPU-callable helpers with$:GPU_ROUTINE(function_name='...', parallelism='[seq]')immediately after declaration
Do not use OpenACC or OpenMP directives directly; use Fypp macros fromsrc/common/include/parallel_macros.fppinstead
Wrap tight loops with$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')macro; addcollapse=nfor safe nested loop merging
Declare loop-local variables withprivate='[...]'in GPU parallel loop macros
Allocate large arrays withmanagedor move them into a persistent$:GPU_ENTER_DATA(...)region at start-up
Do not placestoporerror stopinside device code
Files:
src/simulation/m_ibm.fpp
src/**/*.fpp
📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)
src/**/*.fpp: Use.fppfile extension for Fypp preprocessed files; CMake transpiles them to.f90
Start module files with Fypp include for macros:#:include 'macros.fpp'
Use the fyppASSERTmacro for validating conditions:@:ASSERT(predicate, message)
Use fypp macro@:ALLOCATE(var1, var2)for device-aware allocation instead of standard Fortranallocate
Use fypp macro@:DEALLOCATE(var1, var2)for device-aware deallocation instead of standard Fortrandeallocate
Files:
src/simulation/m_ibm.fpp
🧠 Learnings (21)
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight loops with `$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')` macro; add `collapse=n` for safe nested loop merging
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Mark GPU-callable helpers with `$:GPU_ROUTINE(function_name='...', parallelism='[seq]')` immediately after declaration
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large arrays with `managed` or move them into a persistent `$:GPU_ENTER_DATA(...)` region at start-up
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Do not use OpenACC or OpenMP directives directly; use Fypp macros from `src/common/include/parallel_macros.fpp` instead
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Declare loop-local variables with `private='[...]'` in GPU parallel loop macros
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Avoid stop/error stop inside GPU device code
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Use `private` declaration followed by explicit `public` exports in modules
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to **/*.{fpp,f90} : Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Every variable must have `intent(in|out|inout)` specification and appropriate `dimension` / `allocatable` / `pointer`
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Favor array operations over explicit loops when possible; use `collapse=N` directive to optimize nested loops
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Avoid `goto` statements (except unavoidable legacy); avoid global state (`COMMON`, `save`)
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Each variable should have one clear purpose; do not use the same variable for multiple purposes
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to **/*.{fpp,f90} : Keep subroutine size ≤ 500 lines, helper subroutines ≤ 150 lines, functions ≤ 100 lines, files ≤ 1000 lines
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to **/*.{fpp,f90} : Use 2-space indentation; continuation lines align beneath &
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Indent 2 spaces; continuation lines align under `&`
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Enforce size limits: subroutine ≤ 500 lines, helper ≤ 150, function ≤ 100, module/file ≤ 1000
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Avoid outdated Fortran features: no `equivalence` statements, no `data` statements (use initialization expressions), use `character(len=N)` instead of `Character*N`
Applied to files:
src/simulation/m_ibm.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Use only free-form Fortran (no fixed format); no column-position dependent code
Applied to files:
src/simulation/m_ibm.fpp
🪛 GitHub Actions: Pretty
src/simulation/m_ibm.fpp
[error] 1045-1045: Formatting check failed: line changed from 'local_torque_contribution = local_torque_contribution * cell_volume' to 'local_torque_contribution = local_torque_contribution*cell_volume'. Run './mfc.sh format -j $(nproc)' to fix.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: Coverage Test on CodeCov
- GitHub Check: Code Cleanliness Check
- GitHub Check: Github (ubuntu, mpi, no-debug, false)
- GitHub Check: Github (macos, mpi, debug, false)
- GitHub Check: Github (macos, mpi, no-debug, false)
- GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, no-debug, true)
- GitHub Check: Github (ubuntu, mpi, debug, true)
- GitHub Check: Self Hosted (gpu, omp, frontier)
- GitHub Check: Self Hosted (cpu, none, gt)
- GitHub Check: Github (ubuntu, mpi, debug, false)
- GitHub Check: Self Hosted (cpu, none, frontier)
- GitHub Check: Self Hosted (gpu, acc, frontier)
- GitHub Check: Self Hosted (gpu, omp, gt)
- GitHub Check: Build & Publish
- GitHub Check: Spell Check
🔇 Additional comments (3)
src/simulation/m_ibm.fpp (3)
294-294: LGTM! Correct replacement of cross_product function with s_cross_product subroutine.The change from a function to a subroutine call is appropriate for GPU compatibility and matches the new definition at lines 1154-1163.
Also applies to: 308-308
1136-1136: LGTM! Distance calculation is now correct.The use of
dot_product(vector_to_axis, vector_to_axis)correctly computes the squared distance to the rotation axis, which is what's needed for the moment of inertia calculation.
1154-1163: LGTM! Correct implementation of cross product subroutine.The replacement of the function with a subroutine is appropriate for GPU compatibility. The implementation is correct with proper intent specifications and GPU_ROUTINE macro.
As per coding guidelines for GPU-callable routines.
|
@sbryngelson All of the GT failures were complaining about not being able to open a shared object file, and had nothing to do with any actual code: These need to be rerun. I appear to have lost permission to relaunch jobs from the github UI. Can you rerun these? |
|
Also there are so many comments from AI reviewers on this page that it is beginning to take me minutes to scroll to the bottom sometimes. I can no longer check on my phone because it is too much text to scroll through. Is there a way we can find a way to parse down the number of AI comments in the future? I worry that one of these days soon I will have a PR open that I won't be able to scroll to the bottom of. |
|
hah yes this does seem to be happening for longer sitting PRs. i think you can just remove the requested reviewers yourself. you may have permissions for that yourself. i am looking into your permissions now. |
|
ok you have permissions to rerun runners and change requested reviewers now. you can also just click 'resolve conversation' as a workaround for each comment. i'm tempted to not remove them (or at least wait a bit longer to figure out the best ones) because they find many bugs (and also many false bugs, but I'd rather it find bugs + false positives than have nothing, or fewer for now). |
|
Weird error saying it wasn't able to find |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/documentation/case.md (1)
346-350: Extendmoving_ibmdocumentation to include all supported modes and fix grammatical errors.The new parameter descriptions are incomplete and contain grammatical issues:
- Mode 2 (two-way coupling) is missing. Based on the PR, mode 2 should enable two-way coupling where the fluid applies forces/torques back onto the IB. The current text only documents modes 0 and 1.
- Line 346: "applied forces to the fluid based upon it's own motion" has multiple issues:
- "it's" should be "its" (possessive, not contraction of "it is")
- "applied forces" is awkward; rephrase to "applies forces"
- "will result 1-way" is missing "in": should be "will result in 1-way"
- Clarify that
vel(i)andangular_vel(i)are initial conditions; in mode 1 they remain constant, but in mode 2 they are updated by the time integrator.Apply this diff to address the issues:
- `moving_ibm` sets the method by which movement will be applied to the immersed boundary. Using 0 will result in no movement. Using 1 will result 1-way coupling where the boundary moves at a constant rate and applied forces to the fluid based upon it's own motion. In 1-way coupling, the fluid does not apply forces back onto the IB. + `moving_ibm` sets the method by which movement will be applied to the immersed boundary. Using 0 will result in no movement. Using 1 will result in 1-way coupling where the boundary moves at a constant rate and applies forces to the fluid based upon its own motion; the fluid does not apply forces back onto the IB. Using 2 enables two-way coupling where computed fluid pressure forces and torques are applied to update the IB motion (requires `mass` and optionally moment-of-inertia parameters). - `vel(i)` is the initial linear velocity of the IB in the x, y, z direction for i=1, 2, 3. When `moving_ibm` equals 1, this velocity is constant. + `vel(i)` is the initial linear velocity of the IB in the x, y, z direction for i=1, 2, 3. When `moving_ibm` equals 1, this velocity is constant; when `moving_ibm` equals 2, it is updated by the time integrator. - `angular_vel(i)` is the initial angular velocity of the IB about the x, y, z axes for i=1, 2, 3 in radians per second. When `moving_ibm` equals 1, this angular velocity is constant. + `angular_vel(i)` is the initial angular velocity of the IB about the x, y, z axes for i=1, 2, 3 in radians per second. When `moving_ibm` equals 1, this angular velocity is constant; when `moving_ibm` equals 2, it is updated by the time integrator.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/documentation/case.md(7 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/documentation/case.md
394-394: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
395-395: Hard tabs
Column: 13
(MD010, no-hard-tabs)
396-396: Hard tabs
Column: 12
(MD010, no-hard-tabs)
403-403: Hard tabs
Column: 15
(MD010, no-hard-tabs)
404-404: Hard tabs
Column: 13
(MD010, no-hard-tabs)
405-405: Hard tabs
Column: 16
(MD010, no-hard-tabs)
406-406: Hard tabs
Column: 10
(MD010, no-hard-tabs)
417-417: Hard tabs
Column: 108
(MD010, no-hard-tabs)
418-418: Hard tabs
Column: 172
(MD010, no-hard-tabs)
419-419: Hard tabs
Column: 14
(MD010, no-hard-tabs)
420-420: Hard tabs
Column: 132
(MD010, no-hard-tabs)
420-420: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
421-421: Hard tabs
Column: 99
(MD010, no-hard-tabs)
422-422: Hard tabs
Column: 23
(MD010, no-hard-tabs)
589-589: Hard tabs
Column: 80
(MD010, no-hard-tabs)
590-590: Hard tabs
Column: 61
(MD010, no-hard-tabs)
591-591: Hard tabs
Column: 51
(MD010, no-hard-tabs)
594-594: Hard tabs
Column: 64
(MD010, no-hard-tabs)
595-595: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
596-596: Hard tabs
Column: 78
(MD010, no-hard-tabs)
597-597: Hard tabs
Column: 85
(MD010, no-hard-tabs)
598-598: Hard tabs
Column: 85
(MD010, no-hard-tabs)
599-599: Hard tabs
Column: 75
(MD010, no-hard-tabs)
600-600: Hard tabs
Column: 71
(MD010, no-hard-tabs)
601-601: Hard tabs
Column: 91
(MD010, no-hard-tabs)
602-602: Hard tabs
Column: 91
(MD010, no-hard-tabs)
603-603: Hard tabs
Column: 82
(MD010, no-hard-tabs)
605-605: Hard tabs
Column: 89
(MD010, no-hard-tabs)
606-606: Hard tabs
Column: 74
(MD010, no-hard-tabs)
607-607: Hard tabs
Column: 86
(MD010, no-hard-tabs)
614-614: Hard tabs
Column: 97
(MD010, no-hard-tabs)
615-615: Hard tabs
Column: 55
(MD010, no-hard-tabs)
616-616: Hard tabs
Column: 33
(MD010, no-hard-tabs)
616-616: Hard tabs
Column: 62
(MD010, no-hard-tabs)
792-792: Hard tabs
Column: 30
(MD010, no-hard-tabs)
792-792: Hard tabs
Column: 66
(MD010, no-hard-tabs)
793-793: Hard tabs
Column: 30
(MD010, no-hard-tabs)
794-794: Hard tabs
Column: 13
(MD010, no-hard-tabs)
794-794: Hard tabs
Column: 28
(MD010, no-hard-tabs)
795-795: Hard tabs
Column: 11
(MD010, no-hard-tabs)
795-795: Hard tabs
Column: 24
(MD010, no-hard-tabs)
796-796: Hard tabs
Column: 30
(MD010, no-hard-tabs)
797-797: Hard tabs
Column: 8
(MD010, no-hard-tabs)
797-797: Hard tabs
Column: 28
(MD010, no-hard-tabs)
798-798: Hard tabs
Column: 16
(MD010, no-hard-tabs)
798-798: Hard tabs
Column: 28
(MD010, no-hard-tabs)
798-798: Hard tabs
Column: 31
(MD010, no-hard-tabs)
799-799: Hard tabs
Column: 8
(MD010, no-hard-tabs)
799-799: Hard tabs
Column: 25
(MD010, no-hard-tabs)
800-800: Hard tabs
Column: 9
(MD010, no-hard-tabs)
800-800: Hard tabs
Column: 24
(MD010, no-hard-tabs)
801-801: Hard tabs
Column: 12
(MD010, no-hard-tabs)
801-801: Hard tabs
Column: 26
(MD010, no-hard-tabs)
802-802: Hard tabs
Column: 10
(MD010, no-hard-tabs)
802-802: Hard tabs
Column: 30
(MD010, no-hard-tabs)
802-802: Hard tabs
Column: 32
(MD010, no-hard-tabs)
803-803: Hard tabs
Column: 15
(MD010, no-hard-tabs)
803-803: Hard tabs
Column: 29
(MD010, no-hard-tabs)
803-803: Hard tabs
Column: 31
(MD010, no-hard-tabs)
804-804: Hard tabs
Column: 10
(MD010, no-hard-tabs)
804-804: Hard tabs
Column: 28
(MD010, no-hard-tabs)
804-804: Hard tabs
Column: 31
(MD010, no-hard-tabs)
805-805: Hard tabs
Column: 10
(MD010, no-hard-tabs)
805-805: Hard tabs
Column: 28
(MD010, no-hard-tabs)
805-805: Hard tabs
Column: 31
(MD010, no-hard-tabs)
806-806: Hard tabs
Column: 10
(MD010, no-hard-tabs)
806-806: Hard tabs
Column: 28
(MD010, no-hard-tabs)
806-806: Hard tabs
Column: 31
(MD010, no-hard-tabs)
1049-1049: Hard tabs
Column: 23
(MD010, no-hard-tabs)
1050-1050: Hard tabs
Column: 17
(MD010, no-hard-tabs)
1051-1051: Hard tabs
Column: 20
(MD010, no-hard-tabs)
1052-1052: Hard tabs
Column: 21
(MD010, no-hard-tabs)
1053-1053: Hard tabs
Column: 18
(MD010, no-hard-tabs)
1054-1054: Hard tabs
Column: 14
(MD010, no-hard-tabs)
1055-1055: Hard tabs
Column: 21
(MD010, no-hard-tabs)
1056-1056: Hard tabs
Column: 17
(MD010, no-hard-tabs)
1057-1057: Hard tabs
Column: 17
(MD010, no-hard-tabs)
1058-1058: Hard tabs
Column: 19
(MD010, no-hard-tabs)
1059-1059: Hard tabs
Column: 22
(MD010, no-hard-tabs)
1060-1060: Hard tabs
Column: 20
(MD010, no-hard-tabs)
1061-1061: Hard tabs
Column: 21
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Code Cleanliness Check
- GitHub Check: Github (macos, mpi, debug, false)
- GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
- GitHub Check: Github (macos, mpi, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, debug, true)
- GitHub Check: Github (ubuntu, mpi, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, debug, false)
- GitHub Check: Github (ubuntu, mpi, no-debug, true)
- GitHub Check: Coverage Test on CodeCov
- GitHub Check: Cursor Bugbot
- GitHub Check: Build & Publish
| . | ||
| | Parameter . | Type | Description | | ||
| | ---: . | :----: | :--- | | ||
| | `bc_[x,y,z]%%beg[end]` . | Integer | Beginning [ending] boundary condition in the $[x,y,z]$-direction (negative integer, see table [Boundary Conditions](#boundary-conditions)) | | ||
| | `bc_[x,y,z]%%vb[1,2,3]`‡. | Real | Velocity in the (x,1), (y, 2), (z,3) direction applied to `bc_[x,y,z]%%beg` | | ||
| | `bc_[x,y,z]%%ve[1,2,3]`‡. | Real | Velocity in the (x,1), (y, 2), (z,3) direction applied to `bc_[x,y,z]%%end` | | ||
| | `model_eqns` | Integer | Multicomponent model: [1] $\Gamma/\Pi_\infty$; [2] 5-equation; [3] 6-equation; [4] 4-equation | | ||
| | `alt_soundspeed` * | Logical | Alternate sound speed and $K \nabla \cdot u$ for 5-equation model | | ||
| | `adv_n` | Logical | Solving directly for the number density (in the method of classes) and compute void fraction from the number density | | ||
| | `mpp_lim` | Logical | Mixture physical parameters limits | | ||
| | `mixture_err` | Logical | Mixture properties correction | | ||
| | `time_stepper` | Integer | Runge--Kutta order [1-3] | | ||
| | `adap_dt` | Logical | Strang splitting scheme with adaptive time stepping | | ||
| | `recon_type` | Integer | Reconstruction Type: [1] WENO; [2] MUSCL | | ||
| | `adap_dt_tol` | Real | Tolerance for adaptive time stepping in Strang splitting scheme| | ||
| | `adap_dt_max_iters` | Integer | Max iteration for adaptive time stepping in Strang splitting scheme | | ||
| | `weno_order` | Integer | WENO order [1,3,5] | | ||
| | `weno_eps` | Real | WENO perturbation (avoid division by zero) | | ||
| | `mapped_weno` | Logical | WENO-M (WENO with mapping of nonlinear weights) | | ||
| | `wenoz` | Logical | WENO-Z | | ||
| | `wenoz_q` | Real | WENO-Z power parameter q (only for WENO7) | | ||
| | `teno` | Logical | TENO (Targeted ENO) | | ||
| | `teno_CT` | Real | TENO threshold for smoothness detection | | ||
| | `null_weights` | Logical | Null WENO weights at boundaries | | ||
| | `mp_weno` | Logical | Monotonicity preserving WENO | | ||
| | `muscl_order` | Integer | MUSCL order [1,2] | | ||
| | `muscl_lim` | Integer | MUSCL Slope Limiter: [1] minmod; [2] monotonized central; [3] Van Albada; [4] Van Leer; [5] SUPERBEE | | ||
| | `int_comp` | Logical | THINC Interface Compression | | ||
| | `ic_eps` | Real | Interface compression threshold (default: 1e-4) | | ||
| | `ic_beta` | Real | Interface compression sharpness parameter (default: 1.6) | | ||
| | `riemann_solver` | Integer | Riemann solver algorithm: [1] HLL*; [2] HLLC; [3] Exact*; [4] HLLD (only for MHD) | | ||
| | `low_Mach` | Integer | Low Mach number correction for HLLC Riemann solver: [0] None; [1] Pressure (Chen et al. 2022); [2] Velocity (Thornber et al. 2008) | | ||
| | `avg_state` | Integer | Averaged state evaluation method: [1] Roe average*; [2] Arithmetic mean | | ||
| | `wave_speeds` | Integer | Wave-speed estimation: [1] Direct (Batten et al. 1997); [2] Pressure-velocity* (Toro 1999) | | ||
| | `weno_Re_flux` | Logical | Compute velocity gradient using scalar divergence theorem | | ||
| | `weno_avg` | Logical | Arithmetic mean of left and right, WENO-reconstructed, cell-boundary values | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all hard tab characters (shown as ^I) in the markdown file
cat -A docs/documentation/case.md | grep $'\t' | head -30Repository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
# Check the specific line ranges mentioned in the review comment
echo "=== Lines 387-422 ==="
sed -n '387,422p' docs/documentation/case.md | cat -A | head -20Repository: MFlowCode/MFC
Length of output: 2033
🏁 Script executed:
# Count hard tabs in each specified range
echo "=== Hard tabs in range 387-422 ==="
sed -n '387,422p' docs/documentation/case.md | grep -c $'\t'
echo "=== Hard tabs in range 587-638 ==="
sed -n '587,638p' docs/documentation/case.md | grep -c $'\t'
echo "=== Hard tabs in range 790-806 ==="
sed -n '790,806p' docs/documentation/case.md | grep -c $'\t'
echo "=== Hard tabs in range 1047-1069 ==="
sed -n '1047,1069p' docs/documentation/case.md | grep -c $'\t'Repository: MFlowCode/MFC
Length of output: 210
Replace hard tabs with spaces in markdown tables for linting compliance.
Hard-tab characters are present in the markdown tables across multiple sections and break markdown linting. Replace all hard tabs with spaces (typically 2–4 spaces for alignment) in the following ranges:
- Lines 387–422 (Simulation Algorithm table): 12 hard tabs
- Lines 587–638 (Formatted Output table): 18 hard tabs
- Lines 790–806 (Ensemble-Averaged Bubble Model table): 15 hard tabs
- Lines 1047–1069 (Patch types table): 13 hard tabs
Most text editors can replace \t with spaces automatically using find-and-replace.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
394-394: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
395-395: Hard tabs
Column: 13
(MD010, no-hard-tabs)
396-396: Hard tabs
Column: 12
(MD010, no-hard-tabs)
403-403: Hard tabs
Column: 15
(MD010, no-hard-tabs)
404-404: Hard tabs
Column: 13
(MD010, no-hard-tabs)
405-405: Hard tabs
Column: 16
(MD010, no-hard-tabs)
406-406: Hard tabs
Column: 10
(MD010, no-hard-tabs)
417-417: Hard tabs
Column: 108
(MD010, no-hard-tabs)
418-418: Hard tabs
Column: 172
(MD010, no-hard-tabs)
419-419: Hard tabs
Column: 14
(MD010, no-hard-tabs)
420-420: Hard tabs
Column: 132
(MD010, no-hard-tabs)
420-420: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
421-421: Hard tabs
Column: 99
(MD010, no-hard-tabs)
422-422: Hard tabs
Column: 23
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
docs/documentation/case.md lines 387-422, 587-638, 790-806, 1047-1069: Markdown
tables in these ranges contain hard-tab characters which break markdown linting;
replace every '\t' with spaces (use 2–4 spaces per original tab to preserve
column alignment), fix any misaligned columns after replacement, and re-run the
markdown linter/preview to ensure tables render correctly.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| if (patch_ib(ib_marker)%geometry == 2) then ! circle | ||
| patch_ib(ib_marker)%moment = 0.5*patch_ib(ib_marker)%mass*(patch_ib(ib_marker)%radius)**2 | ||
| elseif (patch_ib(ib_marker)%geometry == 3) then ! rectangle | ||
| patch_ib(ib_marker)%moment = patch_ib(ib_marker)%mass*(patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2)/6._wp |
There was a problem hiding this comment.
Bug: Rectangle moment of inertia formula uses wrong divisor
The moment of inertia for a rectangle about its center (perpendicular to the plane) is (1/12) * m * (a² + b²), but the formula uses /6._wp giving twice the correct value. This causes incorrect rotational dynamics for rectangular immersed boundaries with two-way coupling.
| integer :: i, j, k, count | ||
| if (p == 0) then | ||
| axis = [0, 1, 0] |
There was a problem hiding this comment.
Bug: 2D rotation axis set to wrong direction
For 2D simulations (p == 0), the rotation axis is set to [0, 1, 0] (y-axis), but 2D objects in the x-y plane rotate about the z-axis [0, 0, 1]. This causes incorrect moment of inertia calculations for non-analytic 2D geometries using the numerical integration path.
| patch_ib(ib_marker)%moment = 1._wp | ||
| return | ||
| else | ||
| axis = axis/sqrt(sum(axis)) |
There was a problem hiding this comment.
Bug: Axis normalization corrupts caller's angular velocity vector
The axis parameter lacks intent(in), so the normalization axis = axis/sqrt(sum(axis)) modifies the caller's variable. Since s_compute_moment_of_inertia is called with patch_ib(i)%angular_vel, this overwrites the angular velocity to a unit vector, destroying its magnitude. The subsequent division by moment in the time stepper then produces incorrect angular velocity values.
| $:END_GPU_PARALLEL_LOOP() | ||
| ! write the final moment assuming the points are all uniform density | ||
| patch_ib(ib_marker)%moment = moment*patch_ib(ib_marker)%mass/(count*cell_volume) |
There was a problem hiding this comment.
Bug: Missing MPI reduction in moment of inertia calculation
The numerical moment of inertia calculation loops over local cells (0:m, 0:n, 0:p) and accumulates count and moment without MPI reduction before computing the final result. In parallel runs where an IB spans multiple MPI ranks, each processor computes using only its local portion, yielding incorrect moment values. Unlike s_compute_ib_forces which calls s_mpi_allreduce_vectors_sum, this function lacks similar reduction.
| patch_ib(i)%vel = patch_ib(i)%vel + rk_coef(s, 3)*dt*(patch_ib(i)%force/patch_ib(i)%mass)/rk_coef(s, 4) | ||
|
|
||
| ! update the angular velocity with the torque value | ||
| patch_ib(i)%angular_vel = (patch_ib(i)%angular_vel*patch_ib(i)%moment) + (rk_coef(s, 3)*dt*patch_ib(i)%torque/rk_coef(s, 4)) ! add the torque to the angular momentum |
There was a problem hiding this comment.
Bug: Moment of inertia used before initialization
Line 636 computes angular momentum using patch_ib(i)%moment before s_compute_moment_of_inertia is called on line 637. On the first time step, moment contains the default value dflt_real (-1.e6), so if initial angular velocity is non-zero, the calculation angular_vel * moment produces wildly incorrect angular momentum values that corrupt the simulation.
|
@sbryngelson This PR has been sitting open for a few days. It looks like the last benchmark was canceled. What do we want to do to close this? I would like to get it merged quickly to prevent my future rebase from being painful. |
2 participants
User description
User description
Description
This PR has the contents for an initial 2-way fluid-structure interaction addition to the code, only considering drag due to to pressure. There will be another PR following that adds viscosity in which we include the contributions due to the viscous stress tensor. This PR adds a new method for computing the interior pressure to an IB which reduces to the same method as before when the object is stationary, a method for numerically or analytically setting the moment of inertia, and a way to compute the forces and toques applied to an object from the pressure gradient.
There
Type of change
Please delete options that are not relevant.
Scope
If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.
How Has This Been Tested?
I have run
Test Configuration:
Checklist
docs/)examples/that demonstrate my new feature performing as expected.They run to completion and demonstrate "interesting physics"
./mfc.sh formatbefore committing my codeIf your code changes any code source files (anything in
src/simulation)To make sure the code is performing as expected on GPU devices, I have:
nvtxranges so that they can be identified in profiles./mfc.sh run XXXX --gpu -t simulation --nsys, and have attached the output file (.nsys-rep) and plain text results to this PR./mfc.sh run XXXX --gpu -t simulation --rsys --hip-trace, and have attached the output file and plain text results to this PR.PR Type
Enhancement
Description
Implements two-way fluid-structure interaction via volume integration method
Adds force and torque computation on immersed boundaries from pressure gradients
Introduces moment of inertia calculation with analytical and numerical methods
Extends time-stepping integration to apply computed forces to moving objects
Adds example case demonstrating cylinder in cross-flow with two-way coupling
Diagram Walkthrough
File Walkthrough
5 files
Add force computation and moment of inertia methodsIntegrate force/torque into time-stepping schemeAdd MPI reduction for vector arraysAdd mass, moment, force, torque fields to IB typeInitialize mass and moment of inertia fields3 files
Broadcast IB mass parameter across MPI ranksAdd mass parameter to IB configuration parsingAdd HiPerGator cluster option to bootstrap2 files
New example case for cylinder in cross-flowDocument moving IB parameters and two-way coupling1 files
Add orange color support to shell utilities1 files
Add test metadata for two-way coupling case1 files
CodeAnt-AI Description
Enable inviscid two-way coupling: fluid pressure drives immersed-boundary translation and rotation
What Changed
Impact
✅ Two-way coupled immersed boundaries✅ Fluid pressure drives IB motion✅ Automatic moment-of-inertia estimation for moving objects💡 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:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
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:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
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.
Summary by CodeRabbit
New Features
Examples
Documentation
Tests
Infrastructure
✏️ Tip: You can customize this high-level summary in your review settings.
Note
Enables two-way immersed-boundary coupling by computing pressure-derived forces/torques and updating IB motion each timestep, with new mass/moment fields, MPI reductions, docs, example, and tests.
s_compute_ib_forces) and vector MPI allreduce.vel/angular_veland positions; refresh IB markers/levelsets each stage.ib_patch_parameterswithmass,moment,force,torque; set defaults and broadcast via MPI.docs/documentation/case.mdfor moving-IBM parameters and coupling details; addpatch_ib(*)%masstocase_dicts.pyand MPI broadcast.examples/2D_mibm_cylinder_in_cross_flow/case.pydemonstrating two-way coupling.tests/127A967A/*) for validation.modules.sh; add orange color inutil.sh.Written by Cursor Bugbot for commit 09c7a6d. Configure here.