Skip to content

Conversation

@danieljvickers
Copy link
Member

@danieljvickers danieljvickers commented Dec 3, 2025

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.

  • New feature (non-breaking change which adds functionality)

Scope

  • This PR comprises a set of related changes with a common goal

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:

  • What computers and compilers did you use to test this:

Checklist

  • I have added comments for the new code
  • I added Doxygen docstrings to the new code
  • I have made corresponding changes to the documentation (docs/)
  • I have added regression tests to the test suite so that people can verify in the future that the feature is behaving as expected
  • I have added example cases in examples/ that demonstrate my new feature performing as expected.
    They run to completion and demonstrate "interesting physics"
  • I ran ./mfc.sh format before committing my code
  • New and existing tests pass locally with my changes, including with GPU capability enabled (both NVIDIA hardware with NVHPC compilers and AMD hardware with CRAY compilers) and disabled
  • This PR does not introduce any repeated code (it follows the DRY principle)
  • I cannot think of a way to condense this code and reduce any introduced additional line count

If 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:

  • Checked that the code compiles using NVHPC compilers
  • Checked that the code compiles using CRAY compilers
  • Ran the code on either V100, A100, or H100 GPUs and ensured the new feature performed as expected (the GPU results match the CPU results)
  • Ran the code on MI200+ GPUs and ensure the new features performed as expected (the GPU results match the CPU results)
  • Enclosed the new feature via nvtx ranges so that they can be identified in profiles
  • Ran a Nsight Systems profile using ./mfc.sh run XXXX --gpu -t simulation --nsys, and have attached the output file (.nsys-rep) and plain text results to this PR
  • Ran a Rocprof Systems profile using ./mfc.sh run XXXX --gpu -t simulation --rsys --hip-trace, and have attached the output file and plain text results to this PR.
  • Ran my code using various numbers of different GPUs (1, 2, and 8, for example) in parallel and made sure that the results scale similarly to what happens if you run without the new code/feature

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

flowchart LR
  A["Pressure Field"] -->|"Volume Integration"| B["Force & Torque Computation"]
  B -->|"Apply to IB"| C["Update IB Velocity"]
  C -->|"Moment of Inertia"| D["Update Angular Velocity"]
  D -->|"Advance Time Step"| E["New IB Position"]
Loading

File Walkthrough

Relevant files
Enhancement
5 files
m_ibm.fpp
Add force computation and moment of inertia methods           
+159/-1 
m_time_steppers.fpp
Integrate force/torque into time-stepping scheme                 
+17/-7   
m_mpi_common.fpp
Add MPI reduction for vector arrays                                           
+24/-0   
m_derived_types.fpp
Add mass, moment, force, torque fields to IB type               
+2/-0     
m_global_parameters.fpp
Initialize mass and moment of inertia fields                         
+2/-0     
Configuration changes
3 files
m_mpi_proxy.fpp
Broadcast IB mass parameter across MPI ranks                         
+1/-1     
case_dicts.py
Add mass parameter to IB configuration parsing                     
+2/-2     
modules.sh
Add HiPerGator cluster option to bootstrap                             
+2/-1     
Documentation
2 files
case.py
New example case for cylinder in cross-flow                           
+105/-0 
case.md
Document moving IB parameters and two-way coupling             
+170/-161
Formatting
1 files
util.sh
Add orange color support to shell utilities                           
+3/-2     
Tests
1 files
golden-metadata.txt
Add test metadata for two-way coupling case                           
+159/-0 
Additional files
1 files
golden.txt +11/-0   


CodeAnt-AI Description

Enable inviscid two-way coupling: fluid pressure drives immersed-boundary translation and rotation

What Changed

  • Immersed boundaries set for two-way coupling (moving_ibm == 2) now receive forces and torques computed from the local pressure field via a volume-integration method; those forces and torques are applied each time step so objects translate and rotate in response to the fluid.
  • Moment of inertia is provided analytically for common shapes and can be estimated from the immersed-boundary interior for arbitrary shapes, so applied torques produce correct angular acceleration and updated angular velocity.
  • A new per-patch mass parameter is accepted and broadcast in multi-process runs; forces and torques are summed across MPI ranks so distributed simulations produce consistent IB motion.
  • Configuration and documentation updated: moving_ibm behavior, initial vel(i) and angular_vel(i) parameters are documented; an example case (2D cylinder in cross flow) demonstrating two-way coupling was added.

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:

@codeant-ai ask: Your question here

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

Example

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

Preserve Org Learnings with CodeAnt

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

@codeant-ai: Your feedback here

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

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

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

@codeant-ai: review

Check Your Repository Health

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

Summary by CodeRabbit

  • New Features

    • Enabled two-way fluid–structure coupling with automatic force/torque computation and mass/moment handling; expanded moving-object support.
    • Added a 2D moving-IB cylinder cross-flow example script.
  • Documentation

    • Reworked case documentation tables and clarified moving-boundary parameters, velocities, and angular-velocity descriptions.
  • Tests

    • Added golden test data and metadata for moving-boundary validation.
  • Infrastructure

    • Added HiPerGator system option and a new terminal color for tooling outputs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Note

Other AI code review bot(s) detected

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

Walkthrough

Adds two-way immersed-boundary coupling: new mass/moment/force/torque fields, MPI vector all-reduce and broadcast of mass, pressure-based force/torque accumulation, moment-of-inertia estimation, RK vectorized IB state updates, an example case, docs and tooling updates, and new golden test data.

Changes

Cohort / File(s) Summary
Documentation
docs/documentation/case.md
Reflowed/realigned parameter tables; added moving_ibm, vel(i), and angular_vel(i) descriptions; minor header/format tweaks.
Example
examples/2D_mibm_cylinder_in_cross_flow/case.py
New script building and printing a JSON config for a 2D moving-IBM cylinder cross-flow case.
Derived types
src/common/m_derived_types.fpp
Added mass, moment (real) and force, torque (real(1:3)) to ib_patch_parameters.
MPI utilities
src/common/m_mpi_common.fpp
Added s_mpi_allreduce_vectors_sum to reduce 2D vector arrays across MPI (with MPI_IN_PLACE handling).
Parameter init
src/pre_process/m_global_parameters.fpp
Initialize patch_ib(i)%mass and patch_ib(i)%moment to defaults in pre-process.
IBM core
src/simulation/m_ibm.fpp
Added pressure-based force/torque computation (s_compute_ib_forces), set interior pressure for moving IBs in state correction, and added s_compute_moment_of_inertia estimator.
MPI proxy
src/simulation/m_mpi_proxy.fpp
Include mass in per-patch MPI_BCAST of patch_ib user inputs.
Time steppers
src/simulation/m_time_steppers.fpp
Call s_compute_ib_forces earlier; relax moving_ibm check to > 0; vectorized RK updates for vel, angular_vel, angles; two-way coupling (moving_ibm == 2) applies force/mass and torque/moment updates and recomputes inertia.
Toolchain mappings
toolchain/mfc/run/case_dicts.py
Added "mass": ParamType.REAL to patch_ib real-attribute mappings for PRE_PROCESS and SIMULATION.
Tooling / modules
toolchain/bootstrap/modules.sh, toolchain/util.sh
Added HiPerGator (h) option to modules selection; added ORANGE color and OR alias.
Tests / golden data
tests/127A967A/golden-metadata.txt, tests/127A967A/golden.txt
Added golden test metadata and numeric reference outputs.

Sequence Diagram(s)

sequenceDiagram
    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: request force/torque computation
    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: reduce/broadcast 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 state without momentum feedback
    end
    IBM->>Patch: angles += angular_vel * dt
    Patch->>TS: updated IB state
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • s_compute_ib_forces: verify pressure sampling, integration weights, GPU parallel loops, and MPI reduction ordering/reshape.
  • s_compute_moment_of_inertia: confirm analytic formulas for common geometries and fallback robustness.
  • m_time_steppers: validate RK coefficients, vectorized updates, and F=ma / τ=Iα ordering with fluid updates.
  • s_mpi_allreduce_vectors_sum and MPI_IN_PLACE: ensure dimensions/strides and in-place semantics match callers.
  • MPI_BCAST extension: confirm mass is marshaled consistently across ranks and tools.

Suggested labels

size:XXL

Suggested reviewers

  • sbryngelson
  • wilfonba

Poem

🐰 I hopped through pressure, torque, and mass,

Felt forces push and moments pass.
I tallied spins with tiny paws,
Angles turn without a pause.
Two-way dance — a rabbit's engineering laugh!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive The description is incomplete. Key sections like testing details, GPU validation, and several checklist items are left unfilled or truncated (e.g., 'I have run' is unfinished), and the testing configuration lacks specifics. Complete the testing section by detailing test cases, configurations, and results. Fill in all incomplete sentences and provide specific compiler/GPU testing details before merging.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the primary change: implementing two-way fluid-structure interaction for inviscid (pressure-driven) coupling with immersed boundaries.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai
Copy link

codeant-ai bot commented Dec 3, 2025

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 ·
Reddit ·
LinkedIn

@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

Pressure divergence uses cell-center coordinates as grid spacing (e.g., dividing by x_cc(i), y_cc(j), z_cc(k)) which is dimensionally incorrect and will blow up near zero; it should use grid spacing (dx, dy, dz). Also index access at i-1/i+1 etc. inside IB cells risks out-of-bounds near domain/IB edges without guards.

! TODO :: This is currently only valid inviscid, and needs to be extended to add viscocity
$:GPU_PARALLEL_LOOP(private='[ib_idx,radial_vector,pressure_divergence,cell_volume]', copy='[forces,torques]', copyin='[ib_markers]', collapse=3)
do i = 0, m
    do j = 0, n
        do k = 0, p
            ib_idx = ib_markers%sf(i, j, k)
            if (ib_idx /= 0) then ! only need to compute the gradient for cells inside a IB
                if (patch_ib(ib_idx)%moving_ibm == 2) then
                    if (num_dims == 3) then
                        radial_vector = [x_cc(i), y_cc(j), z_cc(k)] - [patch_ib(ib_idx)%x_centroid, patch_ib(ib_idx)%y_centroid, patch_ib(ib_idx)%z_centroid] ! get the vector pointing to the grid cell
                    else 
                        radial_vector = [x_cc(i), y_cc(j), 0._wp] - [patch_ib(ib_idx)%x_centroid, patch_ib(ib_idx)%y_centroid, 0._wp] ! get the vector pointing to the grid cell
                    end if
                    pressure_divergence(1) = (pressure(i+1, j, k) - pressure(i-1, j, k)) / (2._wp * x_cc(i))
                    pressure_divergence(2) = (pressure(i, j+1, k) - pressure(i, j-1, k)) / (2._wp * y_cc(j))
                    cell_volume = x_cc(i) * y_cc(j)
                    if (num_dims == 3) then
                        pressure_divergence(3) = (pressure(i, j, k+1) - pressure(i, j, k-1)) / (2._wp * z_cc(k))
                        cell_volume = cell_volume * z_cc(k)
                    else
                        pressure_divergence(3) = 0._wp
                    end if
Possible Issue

s_compute_moment_of_inertia contains several issues: assigns to optional 'axis' argument, uses sqrt(sum(axis)) instead of sqrt(sum(axis**2)), uses 'sum(vector_to_axis)' instead of dot for squared distance, typo 'do j = 0, j' loop, and references 'patch_ib(i)%mass' instead of 'patch_ib(ib_marker)%mass'.

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

! if the IB is in 2D or a 3D sphere, we can compute this exactly
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(i)%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

else ! we do not have an analytic moment of inertia calculation and need to approximate it directly
    count = 0
    moment = 0._wp
    cell_volume = (x_cc(1) - x_cc(0))*(y_cc(1) - y_cc(0)) ! computed without grid stretching. Update in the loop to perform with stretching
    if (p /= 0) then
      cell_volume = cell_volume * (z_cc(1) - z_cc(0))
    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)
    do i = 0, m
        do j = 0, j
            do k = 0, p
                if (ib_markers%sf(i, j, k) == ib_marker) then
                    $:GPU_ATOMIC(atomic='update')
                    count = count + 1 ! increment the count of total cells in the boundary

                    ! get the position in local coordinates so that the axis passes through 0, 0, 0
                    if (p == 0) then
                        position = [x_cc(i), y_cc(j), 0._wp] - [patch_ib(ib_marker)%x_centroid, patch_ib(ib_marker)%y_centroid, 0._wp]
                    else
                        position = [x_cc(i), y_cc(j), z_cc(k)] - [patch_ib(ib_marker)%x_centroid, patch_ib(ib_marker)%y_centroid, patch_ib(ib_marker)%z_centroid]
                    end if

                    ! project the position along the axis to find the closest distance to the rotation axis
                    closest_point_along_axis = axis*dot_product(axis, position)
                    vector_to_axis = position - closest_point_along_axis
                    distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared

                    ! compute the position component of the moment
                    $:GPU_ATOMIC(atomic='update')
                    moment = moment + distance_to_axis
                end if
            end do
        end do
    end do
    $: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)
    $:GPU_UPDATE(device='[patch_ib(ib_marker)%moment]')
end if
API/Physics Consistency

Two-way coupling update mixes angular momentum and angular velocity directly (adds torque to angular velocity times moment, then divides by updated moment). Needs clear use of I*omega integration and consistent frame for torque (already rotated); verify rk_coef usage and units. Also unconditional print of forces elsewhere suggests debug code left in.

    patch_ib(i)%step_z_centroid = patch_ib(i)%z_centroid
end if

if (patch_ib(i)%moving_ibm > 0) then
    patch_ib(i)%vel = (rk_coef(s, 1)*patch_ib(i)%step_vel + rk_coef(s, 2)*patch_ib(i)%vel)/rk_coef(s, 4)
    patch_ib(i)%angular_vel = (rk_coef(s, 1)*patch_ib(i)%step_angular_vel + rk_coef(s, 2)*patch_ib(i)%angular_vel)/rk_coef(s, 4)

    if (patch_ib(i)%moving_ibm == 2) then ! if we are using two-way coupling, apply force and torque
        ! 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)

        ! 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
        call s_compute_moment_of_inertia(i, patch_ib(i)%angular_vel) ! update the moment of inertia to be based on the direction of the angular momentum
        patch_ib(i)%angular_vel = patch_ib(i)%angular_vel / patch_ib(i)%moment ! convert back to angular velocity with the new moment of inertia
    end if

    ! Update the angle of the IB
    patch_ib(i)%angles = (rk_coef(s, 1)*patch_ib(i)%step_angles + rk_coef(s, 2)*patch_ib(i)%angles + rk_coef(s, 3)*patch_ib(i)%angular_vel*dt)/rk_coef(s, 4)

@codeant-ai codeant-ai bot added the size:XL This PR changes 500-999 lines, ignoring generated files label Dec 3, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

High-level Suggestion

The force and torque calculations in s_compute_ib_forces are incorrect because the pressure gradient and cell volume are computed using cell coordinates instead of grid spacing and cell dimensions. This invalidates the fluid-structure interaction results. [High-level, importance: 9]

Solution Walkthrough:

Before:

subroutine s_compute_ib_forces(pressure)
    ...
    do i = 0, m
        do j = 0, n
            do k = 0, p
                ...
                ! Incorrect pressure gradient using cell-center coordinates in denominator
                pressure_divergence(1) = (pressure(i+1,j,k) - pressure(i-1,j,k)) / (2._wp * x_cc(i))
                pressure_divergence(2) = (pressure(i,j+1,k) - pressure(i,j-1,k)) / (2._wp * y_cc(j))
                
                ! Incorrect cell volume calculation using coordinates
                cell_volume = x_cc(i) * y_cc(j)
                if (num_dims == 3) then
                    pressure_divergence(3) = (pressure(i,j,k+1) - pressure(i,j,k-1)) / (2._wp * z_cc(k))
                    cell_volume = cell_volume * z_cc(k)
                end if

                forces(ib_idx, :) = forces(ib_idx, :) - (pressure_divergence * cell_volume)
                ...
            end do
        end do
    end do
end subroutine

After:

subroutine s_compute_ib_forces(pressure)
    ...
    ! Grid spacings (dx, dy, dz) should be available or computed
    ! e.g., dx = x_cc(1) - x_cc(0) for a uniform grid
    
    do i = 0, m
        do j = 0, n
            do k = 0, p
                ...
                ! Correct pressure gradient using grid spacing
                pressure_divergence(1) = (pressure(i+1,j,k) - pressure(i-1,j,k)) / (2._wp * dx)
                pressure_divergence(2) = (pressure(i,j+1,k) - pressure(i,j-1,k)) / (2._wp * dy)
                
                ! Correct cell volume calculation
                cell_volume = dx * dy
                if (num_dims == 3) then
                    pressure_divergence(3) = (pressure(i,j,k+1) - pressure(i,j,k-1)) / (2._wp * dz)
                    cell_volume = cell_volume * dz
                end if

                forces(ib_idx, :) = forces(ib_idx, :) - (pressure_divergence * cell_volume)
                ...
            end do
        end do
    end do
end subroutine

Comment on lines 1025 to 1033
pressure_divergence(1) = (pressure(i+1, j, k) - pressure(i-1, j, k)) / (2._wp * x_cc(i))
pressure_divergence(2) = (pressure(i, j+1, k) - pressure(i, j-1, k)) / (2._wp * y_cc(j))
cell_volume = x_cc(i) * y_cc(j)
if (num_dims == 3) then
pressure_divergence(3) = (pressure(i, j, k+1) - pressure(i, j, k-1)) / (2._wp * z_cc(k))
cell_volume = cell_volume * z_cc(k)
else
pressure_divergence(3) = 0._wp
end if
Copy link
Contributor

@qodo-code-review qodo-code-review bot Dec 3, 2025

Choose a reason for hiding this comment

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

Suggestion: Correct the finite difference calculation for pressure_divergence and cell_volume. The denominators for the gradient should use grid spacing differences, and the volume should be calculated from grid spacings, not cell-center coordinates. [possible issue, importance: 9]

Suggested change
pressure_divergence(1) = (pressure(i+1, j, k) - pressure(i-1, j, k)) / (2._wp * x_cc(i))
pressure_divergence(2) = (pressure(i, j+1, k) - pressure(i, j-1, k)) / (2._wp * y_cc(j))
cell_volume = x_cc(i) * y_cc(j)
if (num_dims == 3) then
pressure_divergence(3) = (pressure(i, j, k+1) - pressure(i, j, k-1)) / (2._wp * z_cc(k))
cell_volume = cell_volume * z_cc(k)
else
pressure_divergence(3) = 0._wp
end if
pressure_divergence(1) = (pressure(i+1, j, k) - pressure(i-1, j, k)) / (x_cc(i+1) - x_cc(i-1))
pressure_divergence(2) = (pressure(i, j+1, k) - pressure(i, j-1, k)) / (y_cc(j+1) - y_cc(j-1))
cell_volume = (x_cc(i) - x_cc(i-1)) * (y_cc(j) - y_cc(j-1))
if (num_dims == 3) then
pressure_divergence(3) = (pressure(i, j, k+1) - pressure(i, j, k-1)) / (z_cc(k+1) - z_cc(k-1))
cell_volume = cell_volume * (z_cc(k) - z_cc(k-1))
else
pressure_divergence(3) = 0._wp
end if

Comment on lines 1076 to 1084
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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Correct the normalization of the axis vector by using the sum of squares of its components (sum(axis**2)) instead of the sum of the components (sum(axis)) to calculate its magnitude. [possible issue, importance: 9]

Suggested change
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
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**2))
end if

Comment on lines 634 to 637
! 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
call s_compute_moment_of_inertia(i, patch_ib(i)%angular_vel) ! update the moment of inertia to be based on the direction of the angular momentum
patch_ib(i)%angular_vel = patch_ib(i)%angular_vel / patch_ib(i)%moment ! convert back to angular velocity with the new moment of inertia
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Refactor the angular velocity update to be physically consistent. The moment of inertia should not be recomputed within the time step based on the updated angular momentum. [possible issue, importance: 8]

Suggested change
! 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
call s_compute_moment_of_inertia(i, patch_ib(i)%angular_vel) ! update the moment of inertia to be based on the direction of the angular momentum
patch_ib(i)%angular_vel = patch_ib(i)%angular_vel / patch_ib(i)%moment ! convert back to angular velocity with the new moment of inertia
! update the angular velocity with the torque value
patch_ib(i)%angular_vel = patch_ib(i)%angular_vel + (rk_coef(s, 3)*dt*patch_ib(i)%torque / patch_ib(i)%moment)/rk_coef(s, 4)
! The moment of inertia should be updated based on the new orientation of the body, which is not done here.
! For a rigid body, the moment of inertia tensor in the body-fixed frame is constant.
! What is needed is to transform it to the lab frame.
! The call to s_compute_moment_of_inertia here seems to be based on a misunderstanding.
! If the intent is to handle shape changes or axis of rotation changes, this needs a more robust implementation.
! For now, removing the re-computation within the time step seems safer.
! call s_compute_moment_of_inertia(i, patch_ib(i)%angular_vel)

D/cons.2.00.000000.dat 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05
D/cons.2.00.000050.dat 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.04999999999998 0.04999999999985 0.04999999999852 0.04999999995641 0.0499999983967 0.04999997952971 0.04999978191689 0.04999796506702 0.04999025889306 0.04998169706106 0.04999611085642 0.04999938528897 0.05000003406488 0.05000000856657 0.05000000100545 0.05000000001462 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.04999999999998 0.04999999999991 0.04999999999923 0.04999999996488 0.04999999803882 0.04999997229677 0.04999967269375 0.04999665422966 0.04997168524811 0.04987327254808 0.04979311624675 0.04995397841543 0.04999594739398 0.05000077788507 0.0500001639964 0.05000002052222 0.05000000142172 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.04999999999997 0.04999999999991 0.04999999998695 0.04999999844259 0.04999997623937 0.04999968873807 0.0499965010374 0.04996504309478 0.04973820835164 0.04886555932451 0.04843778808391 0.04966225373664 0.05002701523979 0.05001309247728 0.0500022467731 0.05000026951288 0.05000001831563 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.04999999999997 0.04999999999993 0.04999999999827 0.04999999908656 0.04999998428982 0.04999977563247 0.04999723010338 0.04997053645753 0.04970923688668 0.04791285240187 0.04046420333642 0.03907204776879 0.04957555210315 0.05051144009196 0.05015540600603 0.05002287714675 0.05000246467859 0.05000017642509 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.04999999999999 0.04999999999998 0.04999999999169 0.04999999947116 0.04999999119532 0.04999986124473 0.04999804330651 0.04997620856266 0.04976347728702 0.04755588730479 0.03688418211879 0.01437060350092 0.02045694202298 0.05422678992115 0.05645452469339 0.05155752111215 0.05020233245261 0.05001961721525 0.05000145488255 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.04999999999994 0.04999999987414 0.04999999764279 0.049999962487 0.04999945230823 0.04999309608248 0.0499266930397 0.04936811276406 0.04440131390219 0.02711307233116 0.01470605861931 0.00087507308674 0.04613735708056 0.06908811263212 0.05544404130184 0.05072523836215 0.05007832371291 0.05000602983079 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000025 0.050000000056 0.05000000091518 0.05000001346286 0.05000017294338 0.05000189222029 0.05001739310026 0.05013022588402 0.05111953845441 0.05069538395107 0.00090001380328 0.00083929051022 0.00083936399812 0.05228187392241 0.05012122851237 0.04995499547067 0.0499919416536 0.04999903814445 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000009 0.0500000001691 0.05000000315234 0.05000004921188 0.05000070158163 0.05000864152319 0.05009005344984 0.05076081742011 0.05691156993021 0.07660369574895 0.05047641812066 0.0008425467421 0.02029819842763 0.03199499351858 0.04415345893543 0.04911822882368 0.04990431464006 0.04999245866319 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000001 0.05000000000005 0.05000000001208 0.05000000075044 0.05000001240977 0.05000018666652 0.05000249132331 0.05002826113878 0.05027356434779 0.05211426405864 0.05984506837099 0.05371160985389 0.02079466022408 0.01733300719257 0.03791279186699 0.04723145096985 0.04970555863263 0.04996803051822 0.04999760643662 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000001 0.05000000000005 0.05000000000011 0.0500000000101 0.05000000129503 0.05000002126439 0.0500002882534 0.05000333083001 0.05003513661765 0.05023318838838 0.05095275596936 0.04991870163798 0.03660642996376 0.03896622693327 0.04747990834842 0.04960725690713 0.04995782245821 0.0499959550632 0.0499997084973 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000001 0.05000000000006 0.05000000000015 0.05000000002912 0.05000000220327 0.05000003139675 0.05000038524923 0.05000377343971 0.05002264459563 0.05006307436213 0.04968943232059 0.04768905480554 0.04844277043544 0.04966184466254 0.04995016077492 0.04999470130797 0.04999951229451 0.04999996574246 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000003 0.05000000000014 0.05000000000127 0.05000000005654 0.05000000265932 0.05000003604149 0.05000031792185 0.05000162231062 0.05000073863141 0.04993019223311 0.04969990123289 0.04982000925175 0.04995949705861 0.04999482397968 0.04999946848261 0.04999995327899 0.04999999687019 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000004 0.05000000000028 0.05000000000267 0.05000000008561 0.05000000249016 0.05000002120161 0.05000008543336 0.04999942946791 0.04999252765673 0.04996559869146 0.04998179773187 0.04999605999322 0.04999954734343 0.04999995482017 0.04999999625434 0.04999999988223 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000006 0.05000000000055 0.05000000000221 0.0500000000624 0.05000000113051 0.05000000270023 0.04999989932472 0.04999925232776 0.0499967450211 0.04999842667202 0.04999967545278 0.04999996543917 0.04999999672522 0.04999999985538 0.04999999999801 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000006 0.05000000000045 0.0500000000012 0.04999999999997 0.04999999978849 0.04999999170712 0.04999993680487 0.04999973756309 0.04999988169797 0.0499999768142 0.04999999773868 0.04999999989704 0.0499999999987 0.04999999999946 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000004 0.05000000000017 0.04999999999968 0.04999999998615 0.04999999948328 0.04999999539019 0.04999998154085 0.04999999211345 0.04999999855465 0.0499999999601 0.04999999999938 0.04999999999966 0.04999999999992 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05000000000001 0.04999999999995 0.04999999999907 0.04999999999818 0.04999999980508 0.04999999888801 0.04999999960797 0.04999999999297 0.04999999999979 0.04999999999987 0.04999999999995 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.04999999999988 0.04999999999938 0.04999999999949 0.04999999999973 0.04999999999979 0.04999999999962 0.04999999999996 0.04999999999998 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.04999999999999 0.04999999999989 0.04999999999987 0.04999999999996 0.04999999999995 0.04999999999995 0.04999999999999 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.04999999999999 0.04999999999998 0.04999999999999 0.04999999999999 0.04999999999999 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05 0.05
D/cons.3.00.000000.dat 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0
D/cons.3.00.000050.dat -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -5e-14 -3.9e-13 -1.67e-12 -3.238e-11 -1.602e-09 -1.685203e-08 -1.3879433e-07 -1.20253567e-06 -2.48090056e-06 4.8065816e-07 2.6775886e-06 6.2848626e-07 5.263255e-08 1.95458e-09 -5.4975e-10 -4.098e-11 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -3e-14 -2.6e-13 -1.37e-12 -5.46e-11 -2.17158e-09 -2.666603e-08 -2.6752521e-07 -2.18830974e-06 -1.752609875e-05 -3.211928825e-05 7.66511127e-06 3.647123626e-05 7.51967815e-06 4.9668817e-07 -1.402219e-08 -1.067843e-08 -9.1592e-10 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -2e-14 -1.4e-13 -7.6e-13 -5.087e-11 -2.22457e-09 -3.063205e-08 -3.5114471e-07 -3.39969244e-06 -2.750061072e-05 -0.00018648476564 -0.00027706630843 9.528364695e-05 0.0003330373763 6.687850733e-05 3.2919989e-06 -1.1214102e-06 -2.2039798e-07 -1.580863e-08 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -3.4e-13 -2.576e-11 -1.8256e-09 -2.796068e-08 -3.5602536e-07 -3.91305267e-06 -3.657325698e-05 -0.00029593131579 -0.00179746420834 -0.00240061641215 0.00159538828075 0.00252336445901 0.00038246941389 4.94108514e-06 -2.013154284e-05 -3.59999296e-06 -2.4640266e-07 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -4.5e-13 -6.37e-12 -1.30309e-09 -2.128751e-08 -3.015354e-07 -3.69717504e-06 -3.81679221e-05 -0.00030819093772 -0.00264159178646 -0.00918696605728 -0.00894502422236 0.00790304543832 0.01264497615098 0.00118139905033 -0.00068990604601 -0.00022691359074 -2.794972064e-05 -2.31274576e-06 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -7e-14 -3e-13 -4.2078e-10 -8.27975e-09 -1.2572284e-07 -1.67158865e-06 -1.902987943e-05 -0.00018185379269 -0.00137772693871 -0.01213731773214 -0.0400116682209 -0.05298121495587 -0.1008045121587 -0.03345851742991 -0.0121939798663 -0.00569391521925 -0.00093531569062 -0.00012955329572 -1.138967415e-05 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -9e-14 -6.51e-12 -1.53741e-09 -2.59862e-08 -3.863481e-07 -4.99058985e-06 -5.467985609e-05 -0.00049818810497 -0.0037464779014 -0.02621611747289 -0.07038919342009 -0.10367757133706 -0.09668251912213 -0.09669098460091 -0.0442989083387 -0.02338333186679 -0.00450880022162 -0.00052879220748 -4.894860962e-05 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -7e-14 -2.8e-13 -4.6039e-10 -8.7796e-09 -1.2999687e-07 -1.67723378e-06 -1.840823211e-05 -0.00016761865249 -0.00130118300606 -0.00833918749171 -0.01798631430591 -0.04032600790647 -0.09705762249572 -0.04967291675366 -0.03372943932347 -0.01048973693236 -0.00142846475316 -0.0001999938414 -1.738735182e-05 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -4.8e-13 -1.298e-11 -1.56446e-09 -2.48467e-08 -3.3932365e-07 -3.9442354e-06 -3.726442649e-05 -0.00029614138144 -0.00115164241236 0.00100102189618 0.01253987189686 0.00539062607243 -0.00901288704113 -0.00876673598871 -0.0026601678922 -0.00033376428405 -4.401176322e-05 -3.49810397e-06 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -8e-14 -4.6e-13 -3.392e-11 -2.01121e-09 -2.84468e-08 -3.2460469e-07 -3.08366754e-06 -2.646324632e-05 -5.620222336e-05 0.00042622892921 0.00311887118677 0.00194634041412 -0.00283639975938 -0.00209858305702 -0.00035444153341 -4.528772689e-05 -5.06617314e-06 -3.7266019e-07 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -3e-14 -1.7e-13 -8.1e-13 -7.562e-11 -2.69897e-09 -3.5334e-08 -4.0434199e-07 -2.3295561e-06 2.16260429e-06 7.769877415e-05 0.00044554496059 0.00013680966491 -0.00037923180177 -0.00023783432061 -3.549254234e-05 -4.57215344e-06 -4.9449213e-07 -3.457382e-08 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -4e-14 -2.9e-13 -2.1e-12 -5.735e-11 -2.09036e-09 -2.262854e-08 -1.1067959e-07 5.1652494e-07 9.72943745e-06 5.266275082e-05 1.053522087e-05 -4.549812061e-05 -2.423801213e-05 -3.11928838e-06 -3.9561964e-07 -4.097067e-08 -2.67848e-09 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -5.2e-13 -3.22e-12 -9.174e-11 -1.36949e-09 -1.14039e-09 5.778961e-08 9.2516025e-07 5.01145346e-06 9.1222982e-07 -4.56887504e-06 -2.06191627e-06 -2.4019976e-07 -2.995784e-08 -2.91654e-09 -9.48e-11 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -3e-13 -3.25e-12 -6.422e-11 1.0899e-10 5.92543e-09 7.997296e-08 4.0541908e-07 6.47887e-08 -3.8675841e-07 -1.507329e-07 -1.65617e-08 -2.00472e-09 -6.73e-11 -3.06e-12 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -2e-14 -1e-13 1.4e-13 1.6e-12 2.8909e-10 5.53166e-09 2.958209e-08 3.83231e-09 -2.821985e-08 -9.97116e-09 -1.01878e-09 -2.377e-11 -1.56e-12 -2.6e-13 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 1.1e-13 9.2e-13 1.43e-12 2.8934e-10 2.0374e-09 1.7924e-10 -1.91671e-09 -5.905e-10 -3.86e-12 -7e-14 -9e-14 -2e-14 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 1e-13 2.9e-13 3.99e-12 8.751e-11 1.344e-11 -9.66e-11 -8.07e-12 -6e-14 -2e-14 -2e-14 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 6e-14 1e-13 1.7e-13 4e-14 -2.9e-13 -7e-14 -3e-14 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 1e-14 1e-14 -1e-14 -0.0 -0.0 -1e-14 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 -0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0
Copy link
Contributor

@qodo-code-review qodo-code-review bot Dec 3, 2025

Choose a reason for hiding this comment

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

Suggestion: Replace all occurrences of negative zero (-0.0) with 0.0 in the test data file. This will improve test robustness by ensuring a canonical string representation for zero. [general, importance: 4]

Suggested change
D/cons.3.00.000050.dat -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -5e-14 -3.9e-13 -1.67e-12 -3.238e-11 -1.602e-09 -1.685203e-08 -1.3879433e-07 -1.20253567e-06 -2.48090056e-06 4.8065816e-07 2.6775886e-06 6.2848626e-07 5.263255e-08 1.95458e-09 -5.4975e-10 -4.098e-11 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -3e-14 -2.6e-13 -1.37e-12 -5.46e-11 -2.17158e-09 -2.666603e-08 -2.6752521e-07 -2.18830974e-06 -1.752609875e-05 -3.211928825e-05 7.66511127e-06 3.647123626e-05 7.51967815e-06 4.9668817e-07 -1.402219e-08 -1.067843e-08 -9.1592e-10 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -2e-14 -1.4e-13 -7.6e-13 -5.087e-11 -2.22457e-09 -3.063205e-08 -3.5114471e-07 -3.39969244e-06 -2.750061072e-05 -0.00018648476564 -0.00027706630843 9.528364695e-05 0.0003330373763 6.687850733e-05 3.2919989e-06 -1.1214102e-06 -2.2039798e-07 -1.580863e-08 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -3.4e-13 -2.576e-11 -1.8256e-09 -2.796068e-08 -3.5602536e-07 -3.91305267e-06 -3.657325698e-05 -0.00029593131579 -0.00179746420834 -0.00240061641215 0.00159538828075 0.00252336445901 0.00038246941389 4.94108514e-06 -2.013154284e-05 -3.59999296e-06 -2.4640266e-07 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -4.5e-13 -6.37e-12 -1.30309e-09 -2.128751e-08 -3.015354e-07 -3.69717504e-06 -3.81679221e-05 -0.00030819093772 -0.00264159178646 -0.00918696605728 -0.00894502422236 0.00790304543832 0.01264497615098 0.00118139905033 -0.00068990604601 -0.00022691359074 -2.794972064e-05 -2.31274576e-06 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -7e-14 -3e-13 -4.2078e-10 -8.27975e-09 -1.2572284e-07 -1.67158865e-06 -1.902987943e-05 -0.00018185379269 -0.00137772693871 -0.01213731773214 -0.0400116682209 -0.05298121495587 -0.1008045121587 -0.03345851742991 -0.0121939798663 -0.00569391521925 -0.00093531569062 -0.00012955329572 -1.138967415e-05 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -9e-14 -6.51e-12 -1.53741e-09 -2.59862e-08 -3.863481e-07 -4.99058985e-06 -5.467985609e-05 -0.00049818810497 -0.0037464779014 -0.02621611747289 -0.07038919342009 -0.10367757133706 -0.09668251912213 -0.09669098460091 -0.0442989083387 -0.02338333186679 -0.00450880022162 -0.00052879220748 -4.894860962e-05 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -7e-14 -2.8e-13 -4.6039e-10 -8.7796e-09 -1.2999687e-07 -1.67723378e-06 -1.840823211e-05 -0.00016761865249 -0.00130118300606 -0.00833918749171 -0.01798631430591 -0.04032600790647 -0.09705762249572 -0.04967291675366 -0.03372943932347 -0.01048973693236 -0.00142846475316 -0.0001999938414 -1.738735182e-05 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -4.8e-13 -1.298e-11 -1.56446e-09 -2.48467e-08 -3.3932365e-07 -3.9442354e-06 -3.726442649e-05 -0.00029614138144 -0.00115164241236 0.00100102189618 0.01253987189686 0.00539062607243 -0.00901288704113 -0.00876673598871 -0.0026601678922 -0.00033376428405 -4.401176322e-05 -3.49810397e-06 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -8e-14 -4.6e-13 -3.392e-11 -2.01121e-09 -2.84468e-08 -3.2460469e-07 -3.08366754e-06 -2.646324632e-05 -5.620222336e-05 0.00042622892921 0.00311887118677 0.00194634041412 -0.00283639975938 -0.00209858305702 -0.00035444153341 -4.528772689e-05 -5.06617314e-06 -3.7266019e-07 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -3e-14 -1.7e-13 -8.1e-13 -7.562e-11 -2.69897e-09 -3.5334e-08 -4.0434199e-07 -2.3295561e-06 2.16260429e-06 7.769877415e-05 0.00044554496059 0.00013680966491 -0.00037923180177 -0.00023783432061 -3.549254234e-05 -4.57215344e-06 -4.9449213e-07 -3.457382e-08 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -4e-14 -2.9e-13 -2.1e-12 -5.735e-11 -2.09036e-09 -2.262854e-08 -1.1067959e-07 5.1652494e-07 9.72943745e-06 5.266275082e-05 1.053522087e-05 -4.549812061e-05 -2.423801213e-05 -3.11928838e-06 -3.9561964e-07 -4.097067e-08 -2.67848e-09 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -5.2e-13 -3.22e-12 -9.174e-11 -1.36949e-09 -1.14039e-09 5.778961e-08 9.2516025e-07 5.01145346e-06 9.1222982e-07 -4.56887504e-06 -2.06191627e-06 -2.4019976e-07 -2.995784e-08 -2.91654e-09 -9.48e-11 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 -6e-14 -3e-13 -3.25e-12 -6.422e-11 1.0899e-10 5.92543e-09 7.997296e-08 4.0541908e-07 6.47887e-08 -3.8675841e-07 -1.507329e-07 -1.65617e-08 -2.00472e-09 -6.73e-11 -3.06e-12 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -2e-14 -1e-13 1.4e-13 1.6e-12 2.8909e-10 5.53166e-09 2.958209e-08 3.83231e-09 -2.821985e-08 -9.97116e-09 -1.01878e-09 -2.377e-11 -1.56e-12 -2.6e-13 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -1e-14 1.1e-13 9.2e-13 1.43e-12 2.8934e-10 2.0374e-09 1.7924e-10 -1.91671e-09 -5.905e-10 -3.86e-12 -7e-14 -9e-14 -2e-14 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 1e-13 2.9e-13 3.99e-12 8.751e-11 1.344e-11 -9.66e-11 -8.07e-12 -6e-14 -2e-14 -2e-14 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 6e-14 1e-13 1.7e-13 4e-14 -2.9e-13 -7e-14 -3e-14 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 1e-14 1e-14 -1e-14 -0.0 -0.0 -1e-14 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 -0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 0.0 0.0 0.0 0.0 0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0
D/cons.3.00.000050.dat 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -5e-14 -3.9e-13 -1.67e-12 -3.238e-11 -1.602e-09 -1.685203e-08 -1.3879433e-07 -1.20253567e-06 -2.48090056e-06 4.8065816e-07 2.6775886e-06 6.2848626e-07 5.263255e-08 1.95458e-09 -5.4975e-10 -4.098e-11 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -3e-14 -2.6e-13 -1.37e-12 -5.46e-11 -2.17158e-09 -2.666603e-08 -2.6752521e-07 -2.18830974e-06 -1.752609875e-05 -3.211928825e-05 7.66511127e-06 3.647123626e-05 7.51967815e-06 4.9668817e-07 -1.402219e-08 -1.067843e-08 -9.1592e-10 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -2e-14 -1.4e-13 -7.6e-13 -5.087e-11 -2.22457e-09 -3.063205e-08 -3.5114471e-07 -3.39969244e-06 -2.750061072e-05 -0.00018648476564 -0.00027706630843 9.528364695e-05 0.0003330373763 6.687850733e-05 3.2919989e-06 -1.1214102e-06 -2.2039798e-07 -1.580863e-08 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 -6e-14 -3.4e-13 -2.576e-11 -1.8256e-09 -2.796068e-08 -3.5602536e-07 -3.91305267e-06 -3.657325698e-05 -0.00029593131579 -0.00179746420834 -0.00240061641215 0.00159538828075 0.00252336445901 0.00038246941389 4.94108514e-06 -2.013154284e-05 -3.59999296e-06 -2.4640266e-07 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 -6e-14 -4.5e-13 -6.37e-12 -1.30309e-09 -2.128751e-08 -3.015354e-07 -3.69717504e-06 -3.81679221e-05 -0.00030819093772 -0.00264159178646 -0.00918696605728 -0.00894502422236 0.00790304543832 0.01264497615098 0.00118139905033 -0.00068990604601 -0.00022691359074 -2.794972064e-05 -2.31274576e-06 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 -7e-14 -3e-13 -4.2078e-10 -8.27975e-09 -1.2572284e-07 -1.67158865e-06 -1.902987943e-05 -0.00018185379269 -0.00137772693871 -0.01213731773214 -0.0400116682209 -0.05298121495587 -0.1008045121587 -0.03345851742991 -0.0121939798663 -0.00569391521925 -0.00093531569062 -0.00012955329572 -1.138967415e-05 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 -9e-14 -6.51e-12 -1.53741e-09 -2.59862e-08 -3.863481e-07 -4.99058985e-06 -5.467985609e-05 -0.00049818810497 -0.0037464779014 -0.02621611747289 -0.07038919342009 -0.10367757133706 -0.09668251912213 -0.09669098460091 -0.0442989083387 -0.02338333186679 -0.00450880022162 -0.00052879220748 -4.894860962e-05 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 -7e-14 -2.8e-13 -4.6039e-10 -8.7796e-09 -1.2999687e-07 -1.67723378e-06 -1.840823211e-05 -0.00016761865249 -0.00130118300606 -0.00833918749171 -0.01798631430591 -0.04032600790647 -0.09705762249572 -0.04967291675366 -0.03372943932347 -0.01048973693236 -0.00142846475316 -0.0001999938414 -1.738735182e-05 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 -6e-14 -4.8e-13 -1.298e-11 -1.56446e-09 -2.48467e-08 -3.3932365e-07 -3.9442354e-06 -3.726442649e-05 -0.00029614138144 -0.00115164241236 0.00100102189618 0.01253987189686 0.00539062607243 -0.00901288704113 -0.00876673598871 -0.0026601678922 -0.00033376428405 -4.401176322e-05 -3.49810397e-06 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 -8e-14 -4.6e-13 -3.392e-11 -2.01121e-09 -2.84468e-08 -3.2460469e-07 -3.08366754e-06 -2.646324632e-05 -5.620222336e-05 0.00042622892921 0.00311887118677 0.00194634041412 -0.00283639975938 -0.00209858305702 -0.00035444153341 -4.528772689e-05 -5.06617314e-06 -3.7266019e-07 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -3e-14 -1.7e-13 -8.1e-13 -7.562e-11 -2.69897e-09 -3.5334e-08 -4.0434199e-07 -2.3295561e-06 2.16260429e-06 7.769877415e-05 0.00044554496059 0.00013680966491 -0.00037923180177 -0.00023783432061 -3.549254234e-05 -4.57215344e-06 -4.9449213e-07 -3.457382e-08 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -4e-14 -2.9e-13 -2.1e-12 -5.735e-11 -2.09036e-09 -2.262854e-08 -1.1067959e-07 5.1652494e-07 9.72943745e-06 5.266275082e-05 1.053522087e-05 -4.549812061e-05 -2.423801213e-05 -3.11928838e-06 -3.9561964e-07 -4.097067e-08 -2.67848e-09 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 -6e-14 -5.2e-13 -3.22e-12 -9.174e-11 -1.36949e-09 -1.14039e-09 5.778961e-08 9.2516025e-07 5.01145346e-06 9.1222982e-07 -4.56887504e-06 -2.06191627e-06 -2.4019976e-07 -2.995784e-08 -2.91654e-09 -9.48e-11 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 -6e-14 -3e-13 -3.25e-12 -6.422e-11 1.0899e-10 5.92543e-09 7.997296e-08 4.0541908e-07 6.47887e-08 -3.8675841e-07 -1.507329e-07 -1.65617e-08 -2.00472e-09 -6.73e-11 -3.06e-12 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -2e-14 -1e-13 1.4e-13 1.6e-12 2.8909e-10 5.53166e-09 2.958209e-08 3.83231e-09 -2.821985e-08 -9.97116e-09 -1.01878e-09 -2.377e-11 -1.56e-12 -2.6e-13 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -1e-14 1.1e-13 9.2e-13 1.43e-12 2.8934e-10 2.0374e-09 1.7924e-10 -1.91671e-09 -5.905e-10 -3.86e-12 -7e-14 -9e-14 -2e-14 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 1e-13 2.9e-13 3.99e-12 8.751e-11 1.344e-11 -9.66e-11 -8.07e-12 -6e-14 -2e-14 -2e-14 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 6e-14 1e-13 1.7e-13 4e-14 -2.9e-13 -7e-14 -3e-14 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 1e-14 1e-14 -1e-14 0.0 0.0 -1e-14 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -0.0

| `model_threshold` | Real | Ray fraction inside the model patch above which the fraction is set to one.|
| `moving_ibm` | Integer | Sets the method used for IB movement. |
| `vel(i)` | Real | Initial velocity of the moving IB in the i-th direction. |
| `angular_vel(i)` | Real | Initial angular velcoity of the moving IB in the i-th direction. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Corrects several typos and grammatical errors in the documentation for moving immersed boundaries to improve clarity. [general, importance: 3]

@@ -0,0 +1,105 @@
import json
import math
Copy link

Choose a reason for hiding this comment

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

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 🚨

Suggested change
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.

import json
import math

Mu = 1.84e-05
Copy link

Choose a reason for hiding this comment

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

Suggestion: Unused variable: Mu is assigned but never referenced; this is dead code and can be removed to avoid misleading readers about intended viscous computations. [possible bug]

Severity Level: Critical 🚨

Suggested change
Mu = 1.84e-05
# removed unused variable 'Mu'
Why it matters? ⭐

The variable Mu is defined but not referenced anywhere in the file after the PR. It's dead code and should be removed to avoid confusion; this is a harmless cleanup that doesn't change behavior.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** examples/2D_mibm_cylinder_in_cross_flow/case.py
**Line:** 4:4
**Comment:**
	*Possible Bug: Unused variable: `Mu` is assigned but never referenced; this is dead code and can be removed to avoid misleading readers about intended viscous computations.

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.

gam_a = 1.4

# Configuring case dictionary
print(
Copy link

Choose a reason for hiding this comment

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

Suggestion: Module-level side effect: the code prints the JSON at import time which causes unexpected output whenever this module is imported; wrap the printing in an if __name__ == "__main__": guard so the JSON is only printed when the script is executed directly. [logic error]

Severity Level: Minor ⚠️

Suggested change
print(
if __name__ == "__main__":
Why it matters? ⭐

The module currently performs a print at import time which is a side effect and can surprise callers who import this module. Wrapping the print in if name == "main": is the conventional, low-risk fix to ensure the JSON is only emitted when the script is executed directly.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** examples/2D_mibm_cylinder_in_cross_flow/case.py
**Line:** 8:8
**Comment:**
	*Logic Error: Module-level side effect: the code prints the JSON at import time which causes unexpected output whenever this module is imported; wrap the printing in an `if __name__ == "__main__":` guard so the JSON is only printed when the script is executed directly.

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.

!! 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
Copy link

Choose a reason for hiding this comment

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

Suggestion: The new force and torque arrays use an explicit lower bound notation dimension(1:3) while many other 3-component vectors in this file use dimension(3) or L(3)/R(3) notation; this inconsistency can confuse callers and increase the risk of indexing mistakes. Use the canonical dimension(3) form for 3-element vectors to match the rest of the module and avoid subtle off-by-bound assumptions. [logic error]

Severity Level: Minor ⚠️

Suggested change
real(wp), dimension(1:3) :: force, torque ! vectors for the computed force and torque values applied to an IB
real(wp), dimension(3) :: force, torque ! vectors for the computed force and torque values applied to an IB
Why it matters? ⭐

Changing dimension(1:3) to dimension(3) makes the declaration consistent with other 3-component vectors in the module and avoids accidental reliance on a non-unit lower bound. It's a cosmetic/clarity improvement with no behavioral impact.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/common/m_derived_types.fpp
**Line:** 336:336
**Comment:**
	*Logic Error: The new `force` and `torque` arrays use an explicit lower bound notation `dimension(1:3)` while many other 3-component vectors in this file use `dimension(3)` or `L(3)/R(3)` notation; this inconsistency can confuse callers and increase the risk of indexing mistakes. Use the canonical `dimension(3)` form for 3-element vectors to match the rest of the module and avoid subtle off-by-bound assumptions.

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.

Comment on lines +479 to +481
integer, intent(in) :: num_vectors, vector_length
real(wp), dimension(:, :), intent(in) :: var_loc
real(wp), dimension(:, :), intent(out) :: var_glb
Copy link

Choose a reason for hiding this comment

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

Suggestion: Intent mismatch for in-place reduction: var_glb is declared with INTENT(OUT) but the subroutine uses MPI_IN_PLACE when the caller passes the same buffer for send and receive; INTENT(OUT) allows the compiler to undefine or reallocate the actual argument before the call which makes in-place MPI operations unsafe. Change var_glb to INTENT(INOUT) so the caller's data is preserved on entry for MPI_IN_PLACE use. [logic error]

Severity Level: Minor ⚠️

Suggested change
integer, intent(in) :: num_vectors, vector_length
real(wp), dimension(:, :), intent(in) :: var_loc
real(wp), dimension(:, :), intent(out) :: var_glb
real(wp), dimension(:, :), intent(inout) :: var_glb
Why it matters? ⭐

This is a valid correctness issue. INTENT(OUT) permits the compiler to treat the actual argument as undefined on entry (and even reallocate/erase it), which makes using MPI_IN_PLACE unsafe when the caller passes the same buffer for send and receive. Changing var_glb to INTENT(INOUT) accurately documents and guarantees the value is preserved on entry and is the correct contract for potential in-place MPI calls.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/common/m_mpi_common.fpp
**Line:** 479:481
**Comment:**
	*Logic Error: Intent mismatch for in-place reduction: `var_glb` is declared with INTENT(OUT) but the subroutine uses MPI_IN_PLACE when the caller passes the same buffer for send and receive; INTENT(OUT) allows the compiler to undefine or reallocate the actual argument before the call which makes in-place MPI operations unsafe. Change `var_glb` to INTENT(INOUT) so the caller's data is preserved on entry for MPI_IN_PLACE use.

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.

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,:)))
Copy link

Choose a reason for hiding this comment

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

Suggestion: Division-by-zero risk when computing modified pressure for moving immersed boundaries: the formula divides by pres_IP (and later by the computed denominator) with no guard, so if pres_IP == 0 (or the denominator becomes 0) this will raise a runtime error or produce NaNs; guard the division and handle the zero/near-zero case before dividing. [possible bug]

Severity Level: Critical 🚨

Suggested change
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,:)))
if (pres_IP == 0._wp) then
! protect against division by zero: fall back to the interpolated pressure
q_prim_vf(E_idx)%sf(j, k, l) = pres_IP
else
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-30_wp) then
! denominator too small, avoid unstable division
q_prim_vf(E_idx)%sf(j, k, l) = pres_IP
else
q_prim_vf(E_idx)%sf(j, k, l) = pres_IP / buf
end if
end if
Why it matters? ⭐

The current expression divides by pres_IP and a denominator that can be zero or very small, which can produce NaNs or runtime errors. Guarding the division (check pres_IP and the computed denominator) is a legitimate safety fix. The proposed improved code prevents unstable divisions and falls back to a safe value.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/simulation/m_ibm.fpp
**Line:** 268:268
**Comment:**
	*Possible Bug: Division-by-zero risk when computing modified pressure for moving immersed boundaries: the formula divides by `pres_IP` (and later by the computed denominator) with no guard, so if `pres_IP == 0` (or the denominator becomes 0) this will raise a runtime error or produce NaNs; guard the division and handle the zero/near-zero case before dividing.

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.

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(i)%mass * (patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2) / 6._wp
Copy link

@codeant-ai codeant-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

Suggestion: Wrong variable used when computing analytic moment for rectangular geometry: the expression uses patch_ib(i)%mass but i is undefined here (should use the subroutine argument ib_marker); this will read a wrong/undefined mass value and produce incorrect result. [logic error]

Severity Level: Minor ⚠️

Suggested change
patch_ib(ib_marker)%moment = patch_ib(i)%mass * (patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2) / 6._wp
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
Why it matters? ⭐

The expression reads patch_ib(i)%mass but there is no loop variable i in this branch — the subroutine parameter is ib_marker. Using i is incorrect and will fetch the wrong struct/mass. Replacing with patch_ib(ib_marker)%mass fixes an obvious logic bug.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/simulation/m_ibm.fpp
**Line:** 1090:1090
**Comment:**
	*Logic Error: Wrong variable used when computing analytic moment for rectangular geometry: the expression uses `patch_ib(i)%mass` but `i` is undefined here (should use the subroutine argument `ib_marker`); this will read a wrong/undefined mass value and produce incorrect result.

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.

$: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)
do i = 0, m
do j = 0, j
Copy link

@codeant-ai codeant-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

Suggestion: Loop bound typo in the moment-of-inertia grid scan: the inner loop is written do j = 0, j which uses the loop variable j as its own bound and is almost certainly incorrect (will either be a no-op or undefined behavior); it should use the grid size (e.g., n) instead. [logic error]

Severity Level: Minor ⚠️

Suggested change
do j = 0, j
do j = 0, n
Why it matters? ⭐

The inner loop bound "do j = 0, j" is clearly a typo and will not iterate over the intended grid (uses the loop variable as its own bound). Changing it to "0, n" (or appropriate grid size) fixes a logic bug that would otherwise make the loop a no-op or unpredictable.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/simulation/m_ibm.fpp
**Line:** 1105:1105
**Comment:**
	*Logic Error: Loop bound typo in the moment-of-inertia grid scan: the inner loop is written `do j = 0, j` which uses the loop variable `j` as its own bound and is almost certainly incorrect (will either be a no-op or undefined behavior); it should use the grid size (e.g., `n`) instead.

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.

if (ib) then
! check if any IBMS are moving, and if so, update the markers, ghost points, levelsets, and levelset norms
if (moving_immersed_boundary_flag) then
call s_compute_ib_forces(q_prim_vf(E_idx)%sf(0:m, 0:n, 0:p)) ! compute the force and torque on the IB from the fluid
Copy link

Choose a reason for hiding this comment

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

Suggestion: Unsafe use of q_prim_vf when computing IB forces: q_prim_vf is only allocated in initialization when .not. igr. Calling s_compute_ib_forces(q_prim_vf(E_idx)%sf(...)) without guarding for igr can dereference an unallocated array and crash when igr is true. Wrap the call in a guard so it's only executed when q_prim_vf is available. [possible bug]

Severity Level: Critical 🚨

Suggested change
call s_compute_ib_forces(q_prim_vf(E_idx)%sf(0:m, 0:n, 0:p)) ! compute the force and torque on the IB from the fluid
if (.not. igr) then
call s_compute_ib_forces(q_prim_vf(E_idx)%sf(0:m, 0:n, 0:p)) ! compute the force and torque on the IB from the fluid
end if
Why it matters? ⭐

This is a valid runtime-safety concern: q_prim_vf is only allocated in initialization when (.not. igr).
The PR added an unconditional call that will dereference q_prim_vf when igr is true, which can crash.
Guarding the call is the right minimal fix — though note the IB block later still passes q_prim_vf into s_ibm_correct_state
unconditionally, so fixing just this line avoids one crash but doesn't fully resolve the broader inconsistency.
The suggestion is correct and actionable.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/simulation/m_time_steppers.fpp
**Line:** 614:614
**Comment:**
	*Possible Bug: Unsafe use of `q_prim_vf` when computing IB forces: `q_prim_vf` is only allocated in initialization when `.not. igr`. Calling `s_compute_ib_forces(q_prim_vf(E_idx)%sf(...))` without guarding for `igr` can dereference an unallocated array and crash when `igr` is true. Wrap the call in a guard so it's only executed when `q_prim_vf` is available.

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
Copy link

codeant-ai bot commented Dec 3, 2025

CodeAnt AI finished reviewing your PR.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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 ]; then is almost certainly wrong. -v expects a variable name, not its value, and this will misbehave (or error) when u_c is unset or holds a system code. If the goal is "prompt only when -c/--computer was 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 the show_help “Options” list. Please add Florida (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, and torque lack 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 do
examples/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) (with moving_ibm = 2, downward vel(2), and mass) are consistent with a 2D cylinder-in-cross-flow example and with the new IBM mass field.
  • Mu and the math import are currently unused; you can drop them or, if desired, compute Re from Mu to keep the case self-consistent.
  • Please double-check that moving_ibm = 2 is the correct flag for the new two-way pressure-driven coupling mode, and that this value is documented alongside the other options in docs/documentation/case.md.
src/common/m_mpi_common.fpp (1)

475-497: Add shape assertions and clarify alias handling in s_mpi_allreduce_vectors_sum

The MPI call pattern is fine, but two improvements would make this safer and more self-documenting:

  • Add Fypp ASSERT checks 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_length in the MPI call.

  • The loc(var_loc) == loc(var_glb) in-place detection relies on the non-standard loc intrinsic 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 with loc and accepts call 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

📥 Commits

Reviewing files that changed from the base of the PR and between ab9a1fd and 7c70fd4.

📒 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 with m_<feature> prefix (e.g., m_transport)
Name public subroutines as s_<verb>_<noun> (e.g., s_compute_flux) and functions as f_<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
Avoid goto statements (except unavoidable legacy); avoid global state (COMMON, save)
Every variable must have intent(in|out|inout) specification and appropriate dimension / allocatable / pointer
Use s_mpi_abort(<msg>) for error termination instead of stop
Use !> style documentation for header comments; follow Doxygen Fortran format with !! @param and !! @return tags
Use implicit none statement in all modules
Use private declaration followed by explicit public exports in modules
Use derived types with pointers for encapsulation (e.g., pointer, dimension(:,:,:) => null())
Use pure and elemental attributes for side-effect-free functions; combine them for array ...

Files:

  • src/simulation/m_mpi_proxy.fpp
  • src/simulation/m_time_steppers.fpp
  • src/pre_process/m_global_parameters.fpp
  • src/common/m_mpi_common.fpp
  • src/common/m_derived_types.fpp
  • 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 from src/common/include/parallel_macros.fpp instead
Wrap tight loops with $:GPU_PARALLEL_FOR(private='[...]', copy='[...]') macro; add collapse=n for safe nested loop merging
Declare loop-local variables with private='[...]' in GPU parallel loop macros
Allocate large arrays with managed or move them into a persistent $:GPU_ENTER_DATA(...) region at start-up
Do not place stop or error stop inside device code

Files:

  • src/simulation/m_mpi_proxy.fpp
  • src/simulation/m_time_steppers.fpp
  • src/simulation/m_ibm.fpp
src/**/*.fpp

📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)

src/**/*.fpp: Use .fpp file extension for Fypp preprocessed files; CMake transpiles them to .f90
Start module files with Fypp include for macros: #:include 'macros.fpp'
Use the fypp ASSERT macro for validating conditions: @:ASSERT(predicate, message)
Use fypp macro @:ALLOCATE(var1, var2) for device-aware allocation instead of standard Fortran allocate
Use fypp macro @:DEALLOCATE(var1, var2) for device-aware deallocation instead of standard Fortran deallocate

Files:

  • src/simulation/m_mpi_proxy.fpp
  • src/simulation/m_time_steppers.fpp
  • src/pre_process/m_global_parameters.fpp
  • src/common/m_mpi_common.fpp
  • src/common/m_derived_types.fpp
  • src/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.txt
  • tests/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.txt
  • 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: 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.fpp
  • 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/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.fpp
  • 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
🧬 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 mass to the MPI broadcast list is consistent with the new field added to ib_patch_parameters in m_derived_types.fpp. The broadcast follows the existing pattern for scalar real(wp) fields.

toolchain/mfc/run/case_dicts.py (2)

118-118: LGTM!

The addition of mass to the patch_ib attribute mappings is correctly typed as ParamType.REAL and follows the existing pattern for other IB parameters.


367-367: LGTM!

Symmetric addition of mass to 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_inertia which 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_forces passes q_prim_vf(E_idx)%sf(0:m, 0:n, 0:p). Confirm that E_idx correctly 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_UPDATE directive with patch_ib(ib_marker)%moment may not work correctly. The entire patch_ib array 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 declaration
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

263-270: Redundant loop and unclear multi-fluid handling.

The loop do q = 1, num_fluids iterates but q is never used in the loop body. The pressure is simply overwritten num_fluids times 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 if

Likely an incorrect or invalid review comment.

toolchain/util.sh (1)

4-10: ORANGE/OR color additions look good

The new ORANGE escape and OR alias 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 reproducibility

The 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 validate

This file is pure reference data (conservative fields and ib_markers) for test 127A967A. 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 IBM mass and moment fields are now defaulted

Initializing patch_ib(i)%mass and patch_ib(i)%moment to dflt_real alongside the other patch_ib fields in s_assign_default_values_to_user_inputs avoids uninitialized state when these aren’t set in a case file and keeps the pre-process side consistent with the extended ib_patch_parameters type.

Comment on lines 292 to +312
| Parameter | Type | Description |
| ---: | :----: | :--- |
| `geometry` | Integer | Geometry configuration of the patch.|
| `x[y,z]_centroid` | Real | Centroid of the applied geometry in the [x,y,z]-direction. |
| `length_x[y,z]` | Real | Length, if applicable, in the [x,y,z]-direction. |
| `radius` | Real | Radius, if applicable, of the applied geometry. |
| `theta` | Real | Angle of attach applied to airfoil IB patches |
| `c` | Real | NACA airfoil parameters (see below) |
| `t` | Real | NACA airfoil parameters (see below) |
| `m` | Real | NACA airfoil parameters (see below) |
| `p` | Real | NACA airfoil parameters (see below) |
| `slip` | Logical | Apply a slip boundary |
| `model_filepath` | String | Path to an STL or OBJ file (not all OBJs are supported). |
| `model_scale(i)` | Real | Model's (applied) scaling factor for component $i$. |
| `model_rotate(i)` | Real | Model's (applied) angle of rotation about axis $i$. |
| `model_translate(i)` | Real | Model's $i$-th component of (applied) translation. |
| `model_spc` | Integer | Number of samples per cell when discretizing the model into the grid. |
| `model_threshold` | Real | Ray fraction inside the model patch above which the fraction is set to one.|
| `geometry` | Integer | Geometry configuration of the patch.|
| `x[y,z]_centroid` | Real | Centroid of the applied geometry in the [x,y,z]-direction. |
| `length_x[y,z]` | Real | Length, if applicable, in the [x,y,z]-direction. |
| `radius` | Real | Radius, if applicable, of the applied geometry. |
| `theta` | Real | Angle of attach applied to airfoil IB patches |
| `c` | Real | NACA airfoil parameters (see below) |
| `t` | Real | NACA airfoil parameters (see below) |
| `m` | Real | NACA airfoil parameters (see below) |
| `p` | Real | NACA airfoil parameters (see below) |
| `slip` | Logical | Apply a slip boundary |
| `model_filepath` | String | Path to an STL or OBJ file (not all OBJs are supported). |
| `model_scale(i)` | Real | Model's (applied) scaling factor for component $i$. |
| `model_rotate(i)` | Real | Model's (applied) angle of rotation about axis $i$. |
| `model_translate(i)` | Real | Model's $i$-th component of (applied) translation. |
| `model_spc` | Integer | Number of samples per cell when discretizing the model into the grid. |
| `model_threshold` | Real | Ray fraction inside the model patch above which the fraction is set to one.|
| `moving_ibm` | Integer | Sets the method used for IB movement. |
| `vel(i)` | Real | Initial velocity of the moving IB in the i-th direction. |
| `angular_vel(i)` | Real | Initial angular velocity of the moving IB in the i-th direction. |
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Document all moving_ibm modes and tighten the new IBM parameter descriptions

  • The new moving_ibm, vel(i), and angular_vel(i) rows are useful, but the text only explains modes 0 and 1. The code and new example use an additional value (e.g., moving_ibm = 2) for two-way, pressure-driven motion. Please extend the description to cover all supported values (0 = fixed, 1 = prescribed/one-way, 2 = two-way coupling with fluid forces/torques) so users can correctly select the new behavior.
  • For clarity, you may also want to state explicitly that vel(i) / angular_vel(i) are initial conditions, and that in mode 1 they remain constant, whereas in mode 2 they are updated by the time integrator based on the computed forces/torques.
  • Minor wording nits while you’re here: fix “applied forces to the fluid based upon it's own motion” → “applies forces to the fluid based upon its own motion”.

Also applies to: 337-341

Comment on lines +1076 to +1084
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
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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 if

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

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

11 issues found across 13 files

Prompt for AI agents (all 11 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="examples/2D_mibm_cylinder_in_cross_flow/case.py">

<violation number="1" location="examples/2D_mibm_cylinder_in_cross_flow/case.py:98">
P2: Incorrect comment: Comment says &quot;z-axis rotation&quot; but this line sets the `mass` parameter. This appears to be a copy-paste error.</violation>
</file>

<file name="docs/documentation/case.md">

<violation number="1" location="docs/documentation/case.md:312">
P2: Typo: &#39;velcoity&#39; should be &#39;velocity&#39;.</violation>

<violation number="2" location="docs/documentation/case.md:339">
P2: Typo: &#39;velcoity&#39; should be &#39;velocity&#39;.</violation>

<violation number="3" location="docs/documentation/case.md:341">
P2: Typo: &#39;velcoity&#39; should be &#39;velocity&#39;.</violation>

<violation number="4" location="docs/documentation/case.md:378">
P1: Stray period characters break markdown table formatting. The `.  ` on a standalone line and appended to table headers will cause rendering issues.</violation>

<violation number="5" location="docs/documentation/case.md:781">
P1: Stray `.  ` characters in table header row break markdown table formatting.</violation>
</file>

<file name="src/simulation/m_ibm.fpp">

<violation number="1" location="src/simulation/m_ibm.fpp:1025">
P1: Pressure gradient calculation uses cell center coordinate `x_cc(i)` instead of grid spacing. Central difference formula should divide by `2*dx` (grid spacing), not the coordinate value `x_cc(i)`.</violation>

<violation number="2" location="src/simulation/m_ibm.fpp:1054">
P2: Debug print statement should be removed from production code.</violation>

<violation number="3" location="src/simulation/m_ibm.fpp:1090">
P1: Wrong variable used: `patch_ib(i)%mass` should be `patch_ib(ib_marker)%mass`. Variable `i` is undefined in this context.</violation>

<violation number="4" location="src/simulation/m_ibm.fpp:1105">
P0: Loop variable `j` is used as its own upper bound. This should be `do j = 0, n` to iterate over the y-dimension grid.</violation>

<violation number="5" location="src/simulation/m_ibm.fpp:1121">
P1: Incorrect distance calculation: `sum(vector_to_axis)` sums components, not computing squared distance. Should be `sum(vector_to_axis**2)` per the comment.</violation>
</file>

Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Ask questions if you need clarification on any suggestion

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

"patch_ib(1)%angular_vel(1)": 0.0, # x-axis rotational velocity in radians per second
"patch_ib(1)%angular_vel(2)": 0.0, # y-axis rotation
"patch_ib(1)%angular_vel(3)": 0.0, # z-axis rotation
"patch_ib(1)%mass": 0.0001, # z-axis rotation
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Incorrect comment: Comment says "z-axis rotation" but this line sets the mass parameter. This appears to be a copy-paste error.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/2D_mibm_cylinder_in_cross_flow/case.py, line 98:

<comment>Incorrect comment: Comment says &quot;z-axis rotation&quot; but this line sets the `mass` parameter. This appears to be a copy-paste error.</comment>

<file context>
@@ -0,0 +1,105 @@
+            &quot;patch_ib(1)%angular_vel(1)&quot;: 0.0,  # x-axis rotational velocity in radians per second
+            &quot;patch_ib(1)%angular_vel(2)&quot;: 0.0,  # y-axis rotation
+            &quot;patch_ib(1)%angular_vel(3)&quot;: 0.0,  # z-axis rotation
+            &quot;patch_ib(1)%mass&quot;: 0.0001,  # z-axis rotation
+            # Fluids Physical Parameters
+            &quot;fluid_pp(1)%gamma&quot;: 1.0e00 / (gam_a - 1.0e00),  # 2.50(Not 1.40)
</file context>
Suggested change
"patch_ib(1)%mass": 0.0001, # z-axis rotation
"patch_ib(1)%mass": 0.0001, # mass of the immersed boundary object
Fix with Cubic


- `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 velcoity 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 velcoity is constant.
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Typo: 'velcoity' should be 'velocity'.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/documentation/case.md, line 341:

<comment>Typo: &#39;velcoity&#39; should be &#39;velocity&#39;.</comment>

<file context>
@@ -331,6 +334,12 @@ Additional details on this specification can be found in [The Naca Airfoil Serie
+
+- `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 velcoity 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 velcoity is constant.
+
 ### 5. Fluid Material’s
</file context>
Suggested change
- `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 velcoity 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.
Fix with Cubic

| `model_threshold` | Real | Ray fraction inside the model patch above which the fraction is set to one.|
| `moving_ibm` | Integer | Sets the method used for IB movement. |
| `vel(i)` | Real | Initial velocity of the moving IB in the i-th direction. |
| `angular_vel(i)` | Real | Initial angular velcoity of the moving IB in the i-th direction. |
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Typo: 'velcoity' should be 'velocity'.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/documentation/case.md, line 312:

<comment>Typo: &#39;velcoity&#39; should be &#39;velocity&#39;.</comment>

<file context>
@@ -291,22 +291,25 @@ This is enabled by adding `&#39;elliptic_smoothing&#39;: &quot;T&quot;,` and `&#39;elliptic_smoothing_
+| `model_threshold`    | Real    | Ray fraction inside the model patch above which the fraction is set to one.|
+| `moving_ibm`         | Integer | Sets the method used for IB movement. |
+| `vel(i)`             | Real    | Initial velocity of the moving IB in the i-th direction. |
+| `angular_vel(i)`     | Real    | Initial angular velcoity of the moving IB in the i-th direction. |
 
 These parameters should be prepended with `patch_ib(j)%` where $j$ is the patch index.
</file context>
Suggested change
| `angular_vel(i)` | Real | Initial angular velcoity of the moving IB in the i-th direction. |
| `angular_vel(i)` | Real | Initial angular velocity of the moving IB in the i-th direction. |
Fix with Cubic


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

- `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 velcoity is constant.
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Typo: 'velcoity' should be 'velocity'.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/documentation/case.md, line 339:

<comment>Typo: &#39;velcoity&#39; should be &#39;velocity&#39;.</comment>

<file context>
@@ -331,6 +334,12 @@ Additional details on this specification can be found in [The Naca Airfoil Serie
 
+- `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&#39;s own motion. In 1-way coupling, the fluid does not apply forces back onto the IB.
+
+- `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 velcoity 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 velcoity is constant.
</file context>
Suggested change
- `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 velcoity 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.
Fix with Cubic

| Parameter | Type | Description |
| ---: | :----: | :--- |
| `bubbles_euler`| Logical | Ensemble-averaged bubble modeling |
| Parameter . | Type | Description |
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P1: Stray . characters in table header row break markdown table formatting.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/documentation/case.md, line 781:

<comment>Stray `.  ` characters in table header row break markdown table formatting.</comment>

<file context>
@@ -769,23 +778,23 @@ Implementation of the parameters into the model follow [Ando (2010)](references.
-| Parameter      | Type    | Description                                    |
-| ---:           | :----:  |          :---                                  |
-| `bubbles_euler`| Logical	| Ensemble-averaged bubble modeling	|
+| Parameter      .  | Type    | Description                                    |
+| ---:           .  | :----:  |          :---                                  |
+| `bubbles_euler`.  | Logical	| Ensemble-averaged bubble modeling	|
</file context>
Suggested change
| Parameter . | Type | Description |
| Parameter | Type | Description |
Fix with Cubic

patch_ib(i)%torque(:) = matmul(patch_ib(i)%rotation_matrix_inverse, torques(i, :)) ! torques must be computed in the local coordinates of the IB
end do
print *, patch_ib(1)%force(1:2)
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Debug print statement should be removed from production code.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1054:

<comment>Debug print statement should be removed from production code.</comment>

<file context>
@@ -969,6 +994,67 @@ contains
+            patch_ib(i)%torque(:) = matmul(patch_ib(i)%rotation_matrix_inverse, torques(i, :)) ! torques must be computed in the local coordinates of the IB
+        end do
+
+        print *, patch_ib(1)%force(1:2)
+
+    end subroutine s_compute_ib_forces
</file context>

✅ Addressed in 0c221ae

else
radial_vector = [x_cc(i), y_cc(j), 0._wp] - [patch_ib(ib_idx)%x_centroid, patch_ib(ib_idx)%y_centroid, 0._wp] ! get the vector pointing to the grid cell
end if
pressure_divergence(1) = (pressure(i+1, j, k) - pressure(i-1, j, k)) / (2._wp * x_cc(i))
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P1: Pressure gradient calculation uses cell center coordinate x_cc(i) instead of grid spacing. Central difference formula should divide by 2*dx (grid spacing), not the coordinate value x_cc(i).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1025:

<comment>Pressure gradient calculation uses cell center coordinate `x_cc(i)` instead of grid spacing. Central difference formula should divide by `2*dx` (grid spacing), not the coordinate value `x_cc(i)`.</comment>

<file context>
@@ -969,6 +994,67 @@ contains
+                            else 
+                                radial_vector = [x_cc(i), y_cc(j), 0._wp] - [patch_ib(ib_idx)%x_centroid, patch_ib(ib_idx)%y_centroid, 0._wp] ! get the vector pointing to the grid cell
+                            end if
+                            pressure_divergence(1) = (pressure(i+1, j, k) - pressure(i-1, j, k)) / (2._wp * x_cc(i))
+                            pressure_divergence(2) = (pressure(i, j+1, k) - pressure(i, j-1, k)) / (2._wp * y_cc(j))
+                            cell_volume = x_cc(i) * y_cc(j)
</file context>

✅ Addressed in cfe6714

! project the position along the axis to find the closest distance to the rotation axis
closest_point_along_axis = axis*dot_product(axis, position)
vector_to_axis = position - closest_point_along_axis
distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P1: Incorrect distance calculation: sum(vector_to_axis) sums components, not computing squared distance. Should be sum(vector_to_axis**2) per the comment.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1121:

<comment>Incorrect distance calculation: `sum(vector_to_axis)` sums components, not computing squared distance. Should be `sum(vector_to_axis**2)` per the comment.</comment>

<file context>
@@ -978,6 +1064,78 @@ contains
+                            ! project the position along the axis to find the closest distance to the rotation axis
+                            closest_point_along_axis = axis*dot_product(axis, position)
+                            vector_to_axis = position - closest_point_along_axis
+                            distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared
+
+                            ! compute the position component of the moment
</file context>
Suggested change
distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared
distance_to_axis = sum(vector_to_axis**2) ! saves the distance to the axis squared

✅ Addressed in a216ed4

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(i)%mass * (patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2) / 6._wp
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P1: Wrong variable used: patch_ib(i)%mass should be patch_ib(ib_marker)%mass. Variable i is undefined in this context.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1090:

<comment>Wrong variable used: `patch_ib(i)%mass` should be `patch_ib(ib_marker)%mass`. Variable `i` is undefined in this context.</comment>

<file context>
@@ -978,6 +1064,78 @@ contains
+        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(i)%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
</file context>
Suggested change
patch_ib(ib_marker)%moment = patch_ib(i)%mass * (patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2) / 6._wp
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

✅ Addressed in a216ed4

$: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)
do i = 0, m
do j = 0, j
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P0: Loop variable j is used as its own upper bound. This should be do j = 0, n to iterate over the y-dimension grid.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1105:

<comment>Loop variable `j` is used as its own upper bound. This should be `do j = 0, n` to iterate over the y-dimension grid.</comment>

<file context>
@@ -978,6 +1064,78 @@ contains
+            
+            $:GPU_PARALLEL_LOOP(private=&#39;[position,closest_point_along_axis,vector_to_axis,distance_to_axis]&#39;, copy=&#39;[moment,count]&#39;, copyin=&#39;[ib_marker,cell_volume,axis]&#39;, collapse=3)
+            do i = 0, m
+                do j = 0, j
+                    do k = 0, p
+                        if (ib_markers%sf(i, j, k) == ib_marker) then
</file context>
Suggested change
do j = 0, j
do j = 0, n

✅ Addressed in a4967c9

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

9 issues found across 13 files

Prompt for AI agents (all 9 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="examples/2D_mibm_cylinder_in_cross_flow/case.py">

<violation number="1" location="examples/2D_mibm_cylinder_in_cross_flow/case.py:98">
P2: Incorrect comment: The comment `# z-axis rotation` is wrong for the `mass` parameter. This appears to be a copy-paste error from the angular velocity comments above.</violation>
</file>

<file name="docs/documentation/case.md">

<violation number="1" location="docs/documentation/case.md:379">
P2: Erroneous `.  ` characters in table cells will corrupt markdown table rendering. Remove the `.  ` from each affected cell.</violation>

<violation number="2" location="docs/documentation/case.md:781">
P2: Erroneous `.  ` characters in table cells will corrupt markdown table rendering. Remove the `.  ` from each affected cell.</violation>
</file>

<file name="src/common/m_mpi_common.fpp">

<violation number="1" location="src/common/m_mpi_common.fpp:495">
P2: Missing non-MPI fallback: when `MFC_MPI` is not defined, `var_glb` is never assigned, leaving it uninitialized. Similar subroutine `s_mpi_allreduce_integer_sum` includes an `#else` block to copy `var_loc` to `var_glb` for serial execution.</violation>
</file>

<file name="src/simulation/m_ibm.fpp">

<violation number="1" location="src/simulation/m_ibm.fpp:1025">
P1: Pressure gradient calculation uses `x_cc(i)` (cell center coordinate) instead of grid spacing `dx(i)` in denominator. For central difference: `(P[i+1]-P[i-1])/(2*dx)`. Similarly, `cell_volume` should use grid spacings `dx(i)*dy(j)`, not coordinates.</violation>

<violation number="2" location="src/simulation/m_ibm.fpp:1054">
P2: Debug print statement should be removed from production code.</violation>

<violation number="3" location="src/simulation/m_ibm.fpp:1090">
P0: Wrong index variable: `patch_ib(i)%mass` should be `patch_ib(ib_marker)%mass`. Variable `i` is undefined at this point in the code path - it&#39;s only used as a loop variable in the numerical approximation section below.</violation>

<violation number="4" location="src/simulation/m_ibm.fpp:1104">
P0: Loop bound error: `do j = 0, j` uses `j` as both loop variable and upper bound. This will cause incorrect iteration since `j` is uninitialized. Should be `do j = 0, n` to iterate over the y-dimension.</violation>

<violation number="5" location="src/simulation/m_ibm.fpp:1120">
P1: Incorrect distance calculation: `sum(vector_to_axis)` computes the sum of components, not the squared distance. For moment of inertia, this should be `sum(vector_to_axis**2)` to get the squared perpendicular distance from the rotation axis.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

"patch_ib(1)%angular_vel(1)": 0.0, # x-axis rotational velocity in radians per second
"patch_ib(1)%angular_vel(2)": 0.0, # y-axis rotation
"patch_ib(1)%angular_vel(3)": 0.0, # z-axis rotation
"patch_ib(1)%mass": 0.0001, # z-axis rotation
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Incorrect comment: The comment # z-axis rotation is wrong for the mass parameter. This appears to be a copy-paste error from the angular velocity comments above.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/2D_mibm_cylinder_in_cross_flow/case.py, line 98:

<comment>Incorrect comment: The comment `# z-axis rotation` is wrong for the `mass` parameter. This appears to be a copy-paste error from the angular velocity comments above.</comment>

<file context>
@@ -0,0 +1,105 @@
+            &quot;patch_ib(1)%angular_vel(1)&quot;: 0.0,  # x-axis rotational velocity in radians per second
+            &quot;patch_ib(1)%angular_vel(2)&quot;: 0.0,  # y-axis rotation
+            &quot;patch_ib(1)%angular_vel(3)&quot;: 0.0,  # z-axis rotation
+            &quot;patch_ib(1)%mass&quot;: 0.0001,  # z-axis rotation
+            # Fluids Physical Parameters
+            &quot;fluid_pp(1)%gamma&quot;: 1.0e00 / (gam_a - 1.0e00),  # 2.50(Not 1.40)
</file context>
Suggested change
"patch_ib(1)%mass": 0.0001, # z-axis rotation
"patch_ib(1)%mass": 0.0001, # mass of the immersed boundary
Fix with Cubic

| Parameter | Type | Description |
| ---: | :----: | :--- |
| `bubbles_euler`| Logical | Ensemble-averaged bubble modeling |
| Parameter . | Type | Description |
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Erroneous . characters in table cells will corrupt markdown table rendering. Remove the . from each affected cell.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/documentation/case.md, line 781:

<comment>Erroneous `.  ` characters in table cells will corrupt markdown table rendering. Remove the `.  ` from each affected cell.</comment>

<file context>
@@ -769,23 +778,23 @@ Implementation of the parameters into the model follow [Ando (2010)](references.
-| Parameter      | Type    | Description                                    |
-| ---:           | :----:  |          :---                                  |
-| `bubbles_euler`| Logical	| Ensemble-averaged bubble modeling	|
+| Parameter      .  | Type    | Description                                    |
+| ---:           .  | :----:  |          :---                                  |
+| `bubbles_euler`.  | Logical	| Ensemble-averaged bubble modeling	|
</file context>
Suggested change
| Parameter . | Type | Description |
| Parameter | Type | Description |
Fix with Cubic

| `igr_iter_solver` | Integer | Solution method for IGR elliptic solve [1] Jacobi [2] Gauss-Seidel |
| `num_igr_iters` | Integer | Number of iterations for for the IGR elliptic solve (default 2) |
.
| Parameter . | Type | Description |
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Erroneous . characters in table cells will corrupt markdown table rendering. Remove the . from each affected cell.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/documentation/case.md, line 379:

<comment>Erroneous `.  ` characters in table cells will corrupt markdown table rendering. Remove the `.  ` from each affected cell.</comment>

<file context>
@@ -366,62 +375,62 @@ Details of implementation of viscosity in MFC can be found in [Coralic (2015)](r
-| `igr_iter_solver`      | Integer | Solution method for IGR elliptic solve [1] Jacobi [2] Gauss-Seidel |
-| `num_igr_iters`        | Integer | Number of iterations for for the IGR elliptic solve (default 2) |
+.  
+| 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)) |
</file context>
Suggested change
| Parameter . | Type | Description |
| Parameter | Type | Description |
Fix with Cubic

mpi_p, MPI_SUM, MPI_COMM_WORLD, ierr)
end if
#endif
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Missing non-MPI fallback: when MFC_MPI is not defined, var_glb is never assigned, leaving it uninitialized. Similar subroutine s_mpi_allreduce_integer_sum includes an #else block to copy var_loc to var_glb for serial execution.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/common/m_mpi_common.fpp, line 495:

<comment>Missing non-MPI fallback: when `MFC_MPI` is not defined, `var_glb` is never assigned, leaving it uninitialized. Similar subroutine `s_mpi_allreduce_integer_sum` includes an `#else` block to copy `var_loc` to `var_glb` for serial execution.</comment>

<file context>
@@ -472,6 +472,30 @@ contains
+                               mpi_p, MPI_SUM, MPI_COMM_WORLD, ierr)
+        end if
+
+#endif
+
+    end subroutine s_mpi_allreduce_vectors_sum
</file context>
Suggested change
#endif
#else
var_glb = var_loc
#endif

✅ Addressed in 8531946

patch_ib(i)%torque(:) = matmul(patch_ib(i)%rotation_matrix_inverse, torques(i, :)) ! torques must be computed in the local coordinates of the IB
end do
print *, patch_ib(1)%force(1:2)
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Debug print statement should be removed from production code.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1054:

<comment>Debug print statement should be removed from production code.</comment>

<file context>
@@ -969,6 +994,67 @@ contains
+            patch_ib(i)%torque(:) = matmul(patch_ib(i)%rotation_matrix_inverse, torques(i, :)) ! torques must be computed in the local coordinates of the IB
+        end do
+
+        print *, patch_ib(1)%force(1:2)
+
+    end subroutine s_compute_ib_forces
</file context>

✅ Addressed in 0c221ae

else
radial_vector = [x_cc(i), y_cc(j), 0._wp] - [patch_ib(ib_idx)%x_centroid, patch_ib(ib_idx)%y_centroid, 0._wp] ! get the vector pointing to the grid cell
end if
pressure_divergence(1) = (pressure(i + 1, j, k) - pressure(i - 1, j, k))/(2._wp*x_cc(i))
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P1: Pressure gradient calculation uses x_cc(i) (cell center coordinate) instead of grid spacing dx(i) in denominator. For central difference: (P[i+1]-P[i-1])/(2*dx). Similarly, cell_volume should use grid spacings dx(i)*dy(j), not coordinates.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1025:

<comment>Pressure gradient calculation uses `x_cc(i)` (cell center coordinate) instead of grid spacing `dx(i)` in denominator. For central difference: `(P[i+1]-P[i-1])/(2*dx)`. Similarly, `cell_volume` should use grid spacings `dx(i)*dy(j)`, not coordinates.</comment>

<file context>
@@ -969,6 +994,67 @@ contains
+                            else
+                                radial_vector = [x_cc(i), y_cc(j), 0._wp] - [patch_ib(ib_idx)%x_centroid, patch_ib(ib_idx)%y_centroid, 0._wp] ! get the vector pointing to the grid cell
+                            end if
+                            pressure_divergence(1) = (pressure(i + 1, j, k) - pressure(i - 1, j, k))/(2._wp*x_cc(i))
+                            pressure_divergence(2) = (pressure(i, j + 1, k) - pressure(i, j - 1, k))/(2._wp*y_cc(j))
+                            cell_volume = x_cc(i)*y_cc(j)
</file context>

✅ Addressed in cfe6714

! project the position along the axis to find the closest distance to the rotation axis
closest_point_along_axis = axis*dot_product(axis, position)
vector_to_axis = position - closest_point_along_axis
distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P1: Incorrect distance calculation: sum(vector_to_axis) computes the sum of components, not the squared distance. For moment of inertia, this should be sum(vector_to_axis**2) to get the squared perpendicular distance from the rotation axis.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1120:

<comment>Incorrect distance calculation: `sum(vector_to_axis)` computes the sum of components, not the squared distance. For moment of inertia, this should be `sum(vector_to_axis**2)` to get the squared perpendicular distance from the rotation axis.</comment>

<file context>
@@ -978,6 +1064,77 @@ contains
+                            ! project the position along the axis to find the closest distance to the rotation axis
+                            closest_point_along_axis = axis*dot_product(axis, position)
+                            vector_to_axis = position - closest_point_along_axis
+                            distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared
+
+                            ! compute the position component of the moment
</file context>
Suggested change
distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared
distance_to_axis = sum(vector_to_axis**2) ! saves the distance to the axis squared

✅ Addressed in a216ed4

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(i)%mass*(patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2)/6._wp
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P0: Wrong index variable: patch_ib(i)%mass should be patch_ib(ib_marker)%mass. Variable i is undefined at this point in the code path - it's only used as a loop variable in the numerical approximation section below.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1090:

<comment>Wrong index variable: `patch_ib(i)%mass` should be `patch_ib(ib_marker)%mass`. Variable `i` is undefined at this point in the code path - it&#39;s only used as a loop variable in the numerical approximation section below.</comment>

<file context>
@@ -978,6 +1064,77 @@ contains
+        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(i)%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
</file context>
Suggested change
patch_ib(ib_marker)%moment = patch_ib(i)%mass*(patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2)/6._wp
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

✅ Addressed in a216ed4

$: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)
do i = 0, m
do j = 0, j
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P0: Loop bound error: do j = 0, j uses j as both loop variable and upper bound. This will cause incorrect iteration since j is uninitialized. Should be do j = 0, n to iterate over the y-dimension.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1104:

<comment>Loop bound error: `do j = 0, j` uses `j` as both loop variable and upper bound. This will cause incorrect iteration since `j` is uninitialized. Should be `do j = 0, n` to iterate over the y-dimension.</comment>

<file context>
@@ -978,6 +1064,77 @@ contains
+
+            $:GPU_PARALLEL_LOOP(private=&#39;[position,closest_point_along_axis,vector_to_axis,distance_to_axis]&#39;, copy=&#39;[moment,count]&#39;, copyin=&#39;[ib_marker,cell_volume,axis]&#39;, collapse=3)
+            do i = 0, m
+                do j = 0, j
+                    do k = 0, p
+                        if (ib_markers%sf(i, j, k) == ib_marker) then
</file context>
Suggested change
do j = 0, j
do j = 0, n

✅ Addressed in a4967c9

@codeant-ai
Copy link

codeant-ai bot commented Dec 4, 2025

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 ·
Reddit ·
LinkedIn

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/simulation/m_ibm.fpp (1)

36-39: Missing public exports for new subroutines.

The subroutines s_compute_ib_forces and s_compute_moment_of_inertia are called from m_time_steppers.fpp but are not listed in the public exports. They will not be accessible outside this module.

 ; public :: s_initialize_ibm_module, &
  s_ibm_setup, &
  s_ibm_correct_state, &
- s_finalize_ibm_module
+ s_finalize_ibm_module, &
+ s_compute_ib_forces, &
+ s_compute_moment_of_inertia
♻️ Duplicate comments (7)
src/simulation/m_ibm.fpp (7)

1032-1034: 3D pressure gradient also uses cell center instead of grid spacing.

-pressure_divergence(3) = (pressure(i, j, k + 1) - pressure(i, j, k - 1))/(2._wp*z_cc(k))
-cell_volume = cell_volume*z_cc(k)
+pressure_divergence(3) = (pressure(i, j, k + 1) - pressure(i, j, k - 1))/(z_cc(k + 1) - z_cc(k - 1))
+cell_volume = cell_volume*dz(k)

1081-1089: Optional argument handling is unsafe and normalization is incorrect.

Multiple issues flagged by previous reviewers:

  1. Assigning to axis when p == 0 is undefined behavior if axis was not passed
  2. Line 1088 uses sqrt(sum(axis)) which sums components, not squares
+real(wp), dimension(3) :: local_axis
+
 if (p == 0) then
-    axis = [0, 1, 0]
-else if (sqrt(sum(axis**2)) == 0) then
+    local_axis = [0._wp, 0._wp, 1._wp]  ! z-axis for 2D rotation
+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 if

1027-1029: Critical bug: Pressure gradient and cell volume use cell centers instead of grid spacing.

This has been flagged by multiple reviewers. The central difference denominator should use grid spacing (dx, dy, dz) or coordinate differences (x_cc(i+1) - x_cc(i-1)), not cell center coordinates.

-pressure_divergence(1) = (pressure(i + 1, j, k) - pressure(i - 1, j, k))/(2._wp*x_cc(i))
-pressure_divergence(2) = (pressure(i, j + 1, k) - pressure(i, j - 1, k))/(2._wp*y_cc(j))
-cell_volume = x_cc(i)*y_cc(j)
+pressure_divergence(1) = (pressure(i + 1, j, k) - pressure(i - 1, j, k))/(x_cc(i + 1) - x_cc(i - 1))
+pressure_divergence(2) = (pressure(i, j + 1, k) - pressure(i, j - 1, k))/(y_cc(j + 1) - y_cc(j - 1))
+cell_volume = dx(i)*dy(j)

1094-1095: Wrong variable: uses patch_ib(i) instead of patch_ib(ib_marker).

Variable i is undefined at this point—it's only declared as a loop variable later. This will read garbage or cause a compile error.

 elseif (patch_ib(ib_marker)%geometry == 3) then ! rectangle
-    patch_ib(ib_marker)%moment = patch_ib(i)%mass*(patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2)/6._wp
+    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

1109-1109: Loop bound error: do j = 0, j uses j as its own bound.

This should be do j = 0, n to iterate over the y-dimension grid.

-do j = 0, j
+do j = 0, n

1125-1125: Incorrect distance calculation: sums components instead of computing squared magnitude.

The comment says "distance to the axis squared" but sum(vector_to_axis) just adds the vector components. Should be sum(vector_to_axis**2).

-distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared
+distance_to_axis = sum(vector_to_axis**2) ! saves the distance to the axis squared

1072-1075: Missing intent specification for axis parameter.

Per coding guidelines, every argument must have explicit intent. The axis parameter is declared as optional but lacks an intent specification. Since it's being modified in the 2D case and normalized in 3D, it should be intent(inout).

-real(wp), dimension(3), optional :: axis !< the axis about which we compute the moment. Only required in 3D.
+real(wp), dimension(3), intent(inout), optional :: axis !< the axis about which we compute the moment. Only required in 3D.

As per coding guidelines: "Every variable must have intent(in|out|inout) specification."

🧹 Nitpick comments (1)
src/simulation/m_ibm.fpp (1)

199-212: Missing GPU parallelization for interior pressure loop.

This nested loop modifies q_prim_vf(E_idx)%sf which is GPU data, but lacks GPU parallelization directives. Other similar triple-nested loops in this file use $:GPU_PARALLEL_LOOP. Additionally, the hardcoded pressure value 1._wp lacks physical justification—consider making this a configurable parameter or documenting why this specific value is used.

 ! 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)
         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 do
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c70fd4 and ec0ccf6.

📒 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 with m_<feature> prefix (e.g., m_transport)
Name public subroutines as s_<verb>_<noun> (e.g., s_compute_flux) and functions as f_<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
Avoid goto statements (except unavoidable legacy); avoid global state (COMMON, save)
Every variable must have intent(in|out|inout) specification and appropriate dimension / allocatable / pointer
Use s_mpi_abort(<msg>) for error termination instead of stop
Use !> style documentation for header comments; follow Doxygen Fortran format with !! @param and !! @return tags
Use implicit none statement in all modules
Use private declaration followed by explicit public exports in modules
Use derived types with pointers for encapsulation (e.g., pointer, dimension(:,:,:) => null())
Use pure and elemental attributes 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 from src/common/include/parallel_macros.fpp instead
Wrap tight loops with $:GPU_PARALLEL_FOR(private='[...]', copy='[...]') macro; add collapse=n for safe nested loop merging
Declare loop-local variables with private='[...]' in GPU parallel loop macros
Allocate large arrays with managed or move them into a persistent $:GPU_ENTER_DATA(...) region at start-up
Do not place stop or error stop inside device code

Files:

  • src/simulation/m_ibm.fpp
src/**/*.fpp

📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)

src/**/*.fpp: Use .fpp file extension for Fypp preprocessed files; CMake transpiles them to .f90
Start module files with Fypp include for macros: #:include 'macros.fpp'
Use the fypp ASSERT macro for validating conditions: @:ASSERT(predicate, message)
Use fypp macro @:ALLOCATE(var1, var2) for device-aware allocation instead of standard Fortran allocate
Use fypp macro @:DEALLOCATE(var1, var2) for device-aware deallocation instead of standard Fortran deallocate

Files:

  • src/simulation/m_ibm.fpp
🧠 Learnings (11)
📚 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: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} : 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: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} : 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
⏰ 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). (17)
  • GitHub Check: Coverage Test on CodeCov
  • GitHub Check: Code Cleanliness Check
  • GitHub Check: Github (macos, mpi, no-debug, false)
  • GitHub Check: Github (ubuntu, mpi, debug, false)
  • GitHub Check: Github (macos, mpi, debug, false)
  • GitHub Check: Github (ubuntu, mpi, no-debug, false)
  • GitHub Check: Self Hosted (cpu, none, gt)
  • GitHub Check: Github (ubuntu, mpi, debug, true)
  • GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
  • GitHub Check: Github (ubuntu, mpi, no-debug, true)
  • GitHub Check: Self Hosted (gpu, omp, gt)
  • GitHub Check: Self Hosted (gpu, omp, frontier)
  • GitHub Check: Self Hosted (cpu, none, frontier)
  • GitHub Check: Self Hosted (gpu, acc, frontier)
  • GitHub Check: Self Hosted (gpu, acc, gt)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Build & Publish
🔇 Additional comments (1)
src/simulation/m_ibm.fpp (1)

1137-1138: Ensure derived type member is properly managed in GPU data region.

The syntax $:GPU_UPDATE(device='[patch_ib(ib_marker)%moment]') is valid OpenACC, but derived type members—especially if allocatable or pointers—require explicit data management for portability across nvfortran, gfortran, and Cray compilers. Verify that patch_ib is present in an $:GPU_ENTER_DATA(...) region at start-up, or explicitly manage moment with appropriate data clauses. If moment is an allocatable/pointer member, use separate enter/exit data for it rather than relying on implicit deep-copy behavior.

Comment on lines 1040 to 1043
$:GPU_ATOMIC(atomic='update')
forces(ib_idx, :) = forces(ib_idx, :) - (pressure_divergence*cell_volume)
$:GPU_ATOMIC(atomic='update')
torques(ib_idx, :) = torques(ib_idx, :) + (cross_product(radial_vector, pressure_divergence)*cell_volume)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Atomic update on array slice may have undefined behavior.

GPU atomic operations on array slices forces(ib_idx, :) and torques(ib_idx, :) may not atomically update all elements together. Consider using separate atomic updates for each component.

 ! Update the force values atomically to prevent race conditions
-$:GPU_ATOMIC(atomic='update')
-forces(ib_idx, :) = forces(ib_idx, :) - (pressure_divergence*cell_volume)
-$:GPU_ATOMIC(atomic='update')
-torques(ib_idx, :) = torques(ib_idx, :) + (cross_product(radial_vector, pressure_divergence)*cell_volume)
+$:GPU_ATOMIC(atomic='update')
+forces(ib_idx, 1) = forces(ib_idx, 1) - pressure_divergence(1)*cell_volume
+$:GPU_ATOMIC(atomic='update')
+forces(ib_idx, 2) = forces(ib_idx, 2) - pressure_divergence(2)*cell_volume
+$:GPU_ATOMIC(atomic='update')
+forces(ib_idx, 3) = forces(ib_idx, 3) - pressure_divergence(3)*cell_volume
+! Similar expansion needed for torques

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (7)
src/simulation/m_ibm.fpp (7)

261-270: Loop repeatedly overwrites the same pressure value; only the last iteration persists.

The loop over q = 1, num_fluids overwrites q_prim_vf(E_idx)%sf(j, k, l) in each iteration, so only the value from q == num_fluids is retained. This is wasteful and likely a logic error for multi-fluid cases.

Apply this diff to remove the redundant loop (using the first fluid's properties for now, per the TODO):

-                ! 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 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

Based on past review comments.


1026-1037: Critical: Pressure gradient uses cell-center coordinates instead of grid spacing.

The finite-difference gradient divides by cell-center coordinates (x_cc(i), y_cc(j), z_cc(k)) rather than grid spacing. The correct central-difference formula is (P[i+1] - P[i-1]) / (x_cc(i+1) - x_cc(i-1)) or (P[i+1] - P[i-1]) / (2*dx(i)). Similarly, cell_volume should be dx(i)*dy(j)*dz(k), not the product of coordinates.

Apply this diff to fix the gradient and volume calculations:

-                            ! use a finite difference to compute the 2D components of the gradient of the pressure and cell volume
-                            pressure_divergence(1) = (pressure(i + 1, j, k) - pressure(i - 1, j, k))/(2._wp*x_cc(i))
-                            pressure_divergence(2) = (pressure(i, j + 1, k) - pressure(i, j - 1, k))/(2._wp*y_cc(j))
-                            cell_volume = x_cc(i)*y_cc(j)
+                            ! use a finite difference to compute the 2D components of the gradient of the pressure and cell volume
+                            pressure_divergence(1) = (pressure(i + 1, j, k) - pressure(i - 1, j, k))/(x_cc(i + 1) - x_cc(i - 1))
+                            pressure_divergence(2) = (pressure(i, j + 1, k) - pressure(i, j - 1, k))/(y_cc(j + 1) - y_cc(j - 1))
+                            cell_volume = dx(i)*dy(j)
 
                             ! add the 3D component, if we are working in 3 dimensions
                             if (num_dims == 3) then
-                                pressure_divergence(3) = (pressure(i, j, k + 1) - pressure(i, j, k - 1))/(2._wp*z_cc(k))
-                                cell_volume = cell_volume*z_cc(k)
+                                pressure_divergence(3) = (pressure(i, j, k + 1) - pressure(i, j, k - 1))/(z_cc(k + 1) - z_cc(k - 1))
+                                cell_volume = cell_volume*dz(k)
                             else
                                 pressure_divergence(3) = 0._wp
                             end if

Based on past review comments.


1040-1043: Atomic updates on array slices may not work correctly on all GPU compilers.

The $:GPU_ATOMIC directive is applied to array slices forces(ib_idx, :) and torques(ib_idx, :). Some GPU compilers may not atomically update all elements of a slice together, leading to race conditions. Update each component separately with individual atomic directives.

Apply this diff:

                             ! Update the force values atomically to prevent race conditions
                             $:GPU_ATOMIC(atomic='update')
-                            forces(ib_idx, :) = forces(ib_idx, :) - (pressure_divergence*cell_volume)
+                            forces(ib_idx, 1) = forces(ib_idx, 1) - pressure_divergence(1)*cell_volume
+                            $:GPU_ATOMIC(atomic='update')
+                            forces(ib_idx, 2) = forces(ib_idx, 2) - pressure_divergence(2)*cell_volume
+                            $:GPU_ATOMIC(atomic='update')
+                            forces(ib_idx, 3) = forces(ib_idx, 3) - pressure_divergence(3)*cell_volume
+                            
+                            $:GPU_ATOMIC(atomic='update')
+                            torques(ib_idx, 1) = torques(ib_idx, 1) + cross_product(radial_vector, pressure_divergence)(1)*cell_volume
+                            $:GPU_ATOMIC(atomic='update')
+                            torques(ib_idx, 2) = torques(ib_idx, 2) + cross_product(radial_vector, pressure_divergence)(2)*cell_volume
                             $:GPU_ATOMIC(atomic='update')
-                            torques(ib_idx, :) = torques(ib_idx, :) + (cross_product(radial_vector, pressure_divergence)*cell_volume)
+                            torques(ib_idx, 3) = torques(ib_idx, 3) + cross_product(radial_vector, pressure_divergence)(3)*cell_volume

Based on past review comments.


1074-1089: Optional argument axis lacks intent and is assigned without checking present().

The optional argument axis is declared without an intent specification and is assigned at line 1082 without checking present(axis). Additionally, line 1088 incorrectly normalizes axis by sqrt(sum(axis)) instead of sqrt(sum(axis**2)) — this sums components rather than computing the magnitude.

Apply this diff:

-    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 if

Based on past review comments and coding guidelines requiring explicit intent.


1095-1095: Critical: Wrong variable i used instead of ib_marker.

Line 1095 references patch_ib(i)%mass, but i is undefined in this branch (it's only a loop iterator declared later). This should be patch_ib(ib_marker)%mass.

Apply this diff:

         elseif (patch_ib(ib_marker)%geometry == 3) then ! rectangle
-            patch_ib(ib_marker)%moment = patch_ib(i)%mass*(patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2)/6._wp
+            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

Based on past review comments.


1108-1110: Critical: Loop bound typo — j used as its own upper bound.

Line 1109 is written as do j = 0, j, which uses the loop variable j as its own bound. This will cause incorrect iteration (either a no-op or undefined behavior). It should be do j = 0, n to iterate over the y-dimension.

Apply this diff:

             $: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)
             do i = 0, m
-                do j = 0, j
+                do j = 0, n
                     do k = 0, p

Based on past review comments.


1125-1125: Critical: Distance calculation sums components instead of computing squared distance.

Line 1125 computes distance_to_axis = sum(vector_to_axis), which sums the vector components. For moment-of-inertia calculations, this should be sum(vector_to_axis**2) (the squared distance from the rotation axis), as indicated by the comment.

Apply this diff:

-                            distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared
+                            distance_to_axis = sum(vector_to_axis**2) ! saves the distance to the axis squared

Based on past review comments.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec0ccf6 and a4967c9.

📒 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 with m_<feature> prefix (e.g., m_transport)
Name public subroutines as s_<verb>_<noun> (e.g., s_compute_flux) and functions as f_<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
Avoid goto statements (except unavoidable legacy); avoid global state (COMMON, save)
Every variable must have intent(in|out|inout) specification and appropriate dimension / allocatable / pointer
Use s_mpi_abort(<msg>) for error termination instead of stop
Use !> style documentation for header comments; follow Doxygen Fortran format with !! @param and !! @return tags
Use implicit none statement in all modules
Use private declaration followed by explicit public exports in modules
Use derived types with pointers for encapsulation (e.g., pointer, dimension(:,:,:) => null())
Use pure and elemental attributes 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 from src/common/include/parallel_macros.fpp instead
Wrap tight loops with $:GPU_PARALLEL_FOR(private='[...]', copy='[...]') macro; add collapse=n for safe nested loop merging
Declare loop-local variables with private='[...]' in GPU parallel loop macros
Allocate large arrays with managed or move them into a persistent $:GPU_ENTER_DATA(...) region at start-up
Do not place stop or error stop inside device code

Files:

  • src/simulation/m_ibm.fpp
src/**/*.fpp

📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)

src/**/*.fpp: Use .fpp file extension for Fypp preprocessed files; CMake transpiles them to .f90
Start module files with Fypp include for macros: #:include 'macros.fpp'
Use the fypp ASSERT macro for validating conditions: @:ASSERT(predicate, message)
Use fypp macro @:ALLOCATE(var1, var2) for device-aware allocation instead of standard Fortran allocate
Use fypp macro @:DEALLOCATE(var1, var2) for device-aware deallocation instead of standard Fortran deallocate

Files:

  • src/simulation/m_ibm.fpp
🧠 Learnings (11)
📚 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.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 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: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} : 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
⏰ 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: cubic · AI code reviewer
  • GitHub Check: Github (macos, mpi, no-debug, false)
  • GitHub Check: Self Hosted (gpu, omp, gt)
  • GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
  • GitHub Check: Github (ubuntu, mpi, no-debug, false)
  • GitHub Check: Self Hosted (gpu, acc, gt)
  • GitHub Check: Self Hosted (cpu, none, frontier)
  • GitHub Check: Github (macos, mpi, debug, false)
  • GitHub Check: Self Hosted (cpu, none, gt)
  • GitHub Check: Self Hosted (gpu, acc, frontier)
  • GitHub Check: Github (ubuntu, mpi, debug, false)
  • GitHub Check: Github (ubuntu, mpi, no-debug, true)
  • GitHub Check: Github (ubuntu, mpi, debug, true)
  • GitHub Check: Code Cleanliness Check
  • GitHub Check: Build & Publish

@codeant-ai
Copy link

codeant-ai bot commented Dec 4, 2025

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 ·
Reddit ·
LinkedIn

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (7)
src/common/m_mpi_common.fpp (1)

477-481: Intent mismatch for in-place MPI reduction.

The var_glb parameter is declared intent(out), but when loc(var_loc) == loc(var_glb), the subroutine uses MPI_IN_PLACE (line 488). With intent(out), the compiler may treat the actual argument as undefined on entry, making the in-place operation unsafe. Change to intent(inout).

Apply this diff:

-        real(wp), dimension(:, :), intent(out) :: var_glb
+        real(wp), dimension(:, :), intent(inout) :: var_glb

Based on past review comments.

src/simulation/m_ibm.fpp (6)

261-270: Redundant loop overwrites pressure value.

The loop over q = 1, num_fluids repeatedly overwrites q_prim_vf(E_idx)%sf(j, k, l), retaining only the last fluid's value. Either remove the loop and compute once, or implement proper multi-fluid accumulation as indicated by the TODO comment.

Based on past review comments.


1028-1030: Critical: Pressure gradient uses coordinates instead of grid spacing.

The finite difference divides by cell-center coordinates (x_cc(i), y_cc(j)) instead of grid spacing. Central difference requires: (P[i+1] - P[i-1]) / (x_cc(i+1) - x_cc(i-1)). Similarly, cell_volume should use grid spacings dx(i)*dy(j), not coordinates.

Apply this diff:

-                            pressure_divergence(1) = (pressure(i + 1, j, k) - pressure(i - 1, j, k))/(2._wp*x_cc(i))
-                            pressure_divergence(2) = (pressure(i, j + 1, k) - pressure(i, j - 1, k))/(2._wp*y_cc(j))
-                            cell_volume = x_cc(i)*y_cc(j)
+                            pressure_divergence(1) = (pressure(i + 1, j, k) - pressure(i - 1, j, k))/(x_cc(i + 1) - x_cc(i - 1))
+                            pressure_divergence(2) = (pressure(i, j + 1, k) - pressure(i, j - 1, k))/(y_cc(j + 1) - y_cc(j - 1))
+                            cell_volume = dx(i)*dy(j)

Based on past review comments.


1034-1035: Critical: 3D gradient and volume use coordinates instead of spacing.

The 3D gradient calculation has the same issue: divides by z_cc(k) (coordinate) instead of grid spacing. Volume should multiply by dz(k), not z_cc(k).

Apply this diff:

-                                pressure_divergence(3) = (pressure(i, j, k + 1) - pressure(i, j, k - 1))/(2._wp*z_cc(k))
-                                cell_volume = cell_volume*z_cc(k)
+                                pressure_divergence(3) = (pressure(i, j, k + 1) - pressure(i, j, k - 1))/(z_cc(k + 1) - z_cc(k - 1))
+                                cell_volume = cell_volume*dz(k)

Based on past review comments.


1082-1090: Critical: Unsafe optional argument access and incorrect normalization.

Two issues:

  1. Line 1083 assigns to optional axis without checking present(axis) (undefined behavior if not provided)
  2. Line 1089 normalizes using sqrt(sum(axis)) instead of sqrt(sum(axis**2)) (incorrect magnitude calculation)

Consider declaring a local axis variable and copying from the optional parameter when present:

-        real(wp), dimension(3), optional :: axis
+        real(wp), dimension(3), intent(in), optional :: axis
         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_local
         integer :: i, j, k, count
 
         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) .or. sqrt(sum(axis**2)) < 1.e-16_wp) then
             patch_ib(ib_marker)%moment = 1._wp
             return
         else
-            axis = axis/sqrt(sum(axis))
+            axis_local = axis/sqrt(sum(axis**2))
         end if

Based on past review comments.


1096-1096: Critical: Wrong variable index.

Uses patch_ib(i)%mass but i is undefined here (loop variable declared later). Should be patch_ib(ib_marker)%mass.

Apply this diff:

-            patch_ib(ib_marker)%moment = patch_ib(i)%mass*(patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2)/6._wp
+            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

Based on past review comments.


1126-1126: Critical: Incorrect distance calculation.

sum(vector_to_axis) sums components (x + y + z), not the squared distance. For moment of inertia, this should be sum(vector_to_axis**2) to compute r² = x² + y² + z².

Apply this diff:

-                            distance_to_axis = sum(vector_to_axis) ! saves the distance to the axis squared
+                            distance_to_axis = sum(vector_to_axis**2) ! saves the distance to the axis squared

Based on past review comments.

🧹 Nitpick comments (1)
src/simulation/m_ibm.fpp (1)

199-212: Consider GPU parallelization for nested loop.

This 4-level nested loop lacks GPU parallelization. For src/simulation/**/*.fpp files, tight loops should use the $:GPU_PARALLEL_LOOP macro for performance.

Consider applying this pattern:

+        $:GPU_PARALLEL_LOOP(private='[patch_id,j,k,l]', collapse=3)
         do patch_id = 1, num_ibs
             if (patch_ib(patch_id)%moving_ibm == 2) then
                 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 if
         end do
+        $:END_GPU_PARALLEL_LOOP()

As per coding guidelines for GPU code in simulation modules.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a4967c9 and 8531946.

📒 Files selected for processing (2)
  • src/common/m_mpi_common.fpp (1 hunks)
  • 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 with m_<feature> prefix (e.g., m_transport)
Name public subroutines as s_<verb>_<noun> (e.g., s_compute_flux) and functions as f_<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
Avoid goto statements (except unavoidable legacy); avoid global state (COMMON, save)
Every variable must have intent(in|out|inout) specification and appropriate dimension / allocatable / pointer
Use s_mpi_abort(<msg>) for error termination instead of stop
Use !> style documentation for header comments; follow Doxygen Fortran format with !! @param and !! @return tags
Use implicit none statement in all modules
Use private declaration followed by explicit public exports in modules
Use derived types with pointers for encapsulation (e.g., pointer, dimension(:,:,:) => null())
Use pure and elemental attributes for side-effect-free functions; combine them for array ...

Files:

  • src/simulation/m_ibm.fpp
  • src/common/m_mpi_common.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 from src/common/include/parallel_macros.fpp instead
Wrap tight loops with $:GPU_PARALLEL_FOR(private='[...]', copy='[...]') macro; add collapse=n for safe nested loop merging
Declare loop-local variables with private='[...]' in GPU parallel loop macros
Allocate large arrays with managed or move them into a persistent $:GPU_ENTER_DATA(...) region at start-up
Do not place stop or error stop inside device code

Files:

  • src/simulation/m_ibm.fpp
src/**/*.fpp

📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)

src/**/*.fpp: Use .fpp file extension for Fypp preprocessed files; CMake transpiles them to .f90
Start module files with Fypp include for macros: #:include 'macros.fpp'
Use the fypp ASSERT macro for validating conditions: @:ASSERT(predicate, message)
Use fypp macro @:ALLOCATE(var1, var2) for device-aware allocation instead of standard Fortran allocate
Use fypp macro @:DEALLOCATE(var1, var2) for device-aware deallocation instead of standard Fortran deallocate

Files:

  • src/simulation/m_ibm.fpp
  • src/common/m_mpi_common.fpp
🧠 Learnings (17)
📚 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
  • src/common/m_mpi_common.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} : Avoid stop/error stop inside GPU device code

Applied to files:

  • src/simulation/m_ibm.fpp
  • src/common/m_mpi_common.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
  • 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} : Every variable must have `intent(in|out|inout)` specification and appropriate `dimension` / `allocatable` / `pointer`

Applied to files:

  • src/simulation/m_ibm.fpp
  • 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} : Declare loop-local variables with `private='[...]'` in GPU parallel loop macros

Applied to files:

  • src/simulation/m_ibm.fpp
  • 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} : 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
  • 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} : Each variable should have one clear purpose; do not use the same variable for multiple purposes

Applied to files:

  • src/simulation/m_ibm.fpp
  • 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/simulation/m_ibm.fpp
  • 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} : 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: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} : 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 **/*.{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 **/*.{fpp,f90} : Use `s_mpi_abort(<msg>)` for error termination instead of `stop`

Applied to files:

  • src/common/m_mpi_common.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} : Call s_mpi_abort(<msg>) for errors, never use stop or error stop

Applied to files:

  • src/common/m_mpi_common.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). (14)
  • GitHub Check: Code Cleanliness Check
  • GitHub Check: Coverage Test on CodeCov
  • GitHub Check: Self Hosted (gpu, acc, gt)
  • GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
  • GitHub Check: Github (macos, mpi, no-debug, false)
  • GitHub Check: Github (ubuntu, mpi, debug, false)
  • GitHub Check: Github (ubuntu, mpi, debug, true)
  • GitHub Check: Self Hosted (cpu, none, gt)
  • GitHub Check: Github (ubuntu, mpi, no-debug, false)
  • GitHub Check: Github (macos, mpi, debug, false)
  • GitHub Check: Github (ubuntu, mpi, no-debug, true)
  • GitHub Check: Self Hosted (gpu, omp, gt)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Build & Publish
🔇 Additional comments (1)
src/common/m_mpi_common.fpp (1)

495-496: LGTM! Non-MPI fallback correctly implemented.

The serial execution path correctly assigns var_loc to var_glb, addressing the concern raised in past review comments about missing non-MPI fallback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (4)
tests/127A967A/golden.txt (1)

6-6: Normalize negative zero to positive zero for test robustness.

Line 6 contains many -0.0 values mixed with 0.0 values and small finite magnitudes. For canonical floating-point representation and test stability, replace all occurrences of -0.0 with 0.0. This aligns with the prior review suggestion for similar test data and ensures the test isn't sensitive to sign-of-zero variations that can arise from different numerical implementations.

-D/cons.3.00.000050.dat -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -5e-14 -3.9e-13 -1.67e-12 -3.223e-11 -1.59892e-09 -1.681843e-08 -1.3856516e-07 -1.19948516e-06 -2.46667983e-06 4.7305218e-07 2.66766224e-06 6.2837466e-07 5.273083e-08 1.96152e-09 -5.4952e-10 -4.108e-11 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 -0.0 ...
+D/cons.3.00.000050.dat 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 -5e-14 -3.9e-13 -1.67e-12 -3.223e-11 -1.59892e-09 -1.681843e-08 -1.3856516e-07 -1.19948516e-06 -2.46667983e-06 4.7305218e-07 2.66766224e-06 6.2837466e-07 5.273083e-08 1.96152e-09 -5.4952e-10 -4.108e-11 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 ...

Generate a verification script to apply this normalization:

#!/bin/bash
# Replace all `-0.0` with `0.0` in the golden test file
sed -i 's/ -0\.0 / 0.0 /g' tests/127A967A/golden.txt
# Also handle cases at line start/end
sed -i 's/^-0\.0 /0.0 /' tests/127A967A/golden.txt
sed -i 's/ -0\.0$/ 0.0/' tests/127A967A/golden.txt
src/common/m_mpi_common.fpp (1)

479-481: Intent mismatch for in-place MPI reduction.

var_glb is declared intent(out), but when loc(var_loc) == loc(var_glb) the code uses MPI_IN_PLACE, which requires the buffer content to be valid on entry. With intent(out), the compiler may leave the actual argument undefined before the call, making in-place reduction unsafe.

-        real(wp), dimension(:, :), intent(out) :: var_glb
+        real(wp), dimension(:, :), intent(inout) :: var_glb
src/simulation/m_ibm.fpp (2)

259-269: Division-by-zero risks remain unguarded.

The expression on line 267 divides by pres_IP and patch_ib(patch_id)%mass, and the denominator (1._wp - ...) can also approach zero. Previous reviews flagged this. Guard these divisions:

 ! set the pressure
 if (patch_ib(patch_id)%moving_ibm == 0) 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
-        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
+    if (abs(pres_IP) < 1.e-16_wp .or. abs(patch_ib(patch_id)%mass) < 1.e-16_wp) 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
+            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
+    end if
 end if

1077-1094: Optional argument handling and normalization error.

Two issues in this section:

  1. Line 1079: axis is declared optional but lacks intent specification. Per coding guidelines, every argument must have explicit intent.

  2. Line 1093: Normalization uses sqrt(sum(axis)) which sums components, not their squares. Should be sqrt(sum(axis**2)) to compute the magnitude.

-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 if
🧹 Nitpick comments (1)
src/simulation/m_ibm.fpp (1)

1042-1046: Atomic update on array slices may not be element-wise atomic.

GPU atomic operations on array slices like forces(ib_idx, :) may not atomically update all three components together. Consider expanding to separate atomic updates per component to ensure correctness:

 ! Update the force values atomically to prevent race conditions
-$:GPU_ATOMIC(atomic='update')
-forces(ib_idx, :) = forces(ib_idx, :) - (pressure_divergence*cell_volume)
-$:GPU_ATOMIC(atomic='update')
-torques(ib_idx, :) = torques(ib_idx, :) - (cross_product(radial_vector, pressure_divergence)*cell_volume)
+$:GPU_ATOMIC(atomic='update')
+forces(ib_idx, 1) = forces(ib_idx, 1) - pressure_divergence(1)*cell_volume
+$:GPU_ATOMIC(atomic='update')
+forces(ib_idx, 2) = forces(ib_idx, 2) - pressure_divergence(2)*cell_volume
+$:GPU_ATOMIC(atomic='update')
+forces(ib_idx, 3) = forces(ib_idx, 3) - pressure_divergence(3)*cell_volume
+torque_contrib = cross_product(radial_vector, pressure_divergence)*cell_volume
+$:GPU_ATOMIC(atomic='update')
+torques(ib_idx, 1) = torques(ib_idx, 1) - torque_contrib(1)
+$:GPU_ATOMIC(atomic='update')
+torques(ib_idx, 2) = torques(ib_idx, 2) - torque_contrib(2)
+$:GPU_ATOMIC(atomic='update')
+torques(ib_idx, 3) = torques(ib_idx, 3) - torque_contrib(3)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8531946 and a216ed4.

📒 Files selected for processing (5)
  • examples/2D_mibm_cylinder_in_cross_flow/case.py (1 hunks)
  • src/common/m_mpi_common.fpp (1 hunks)
  • src/simulation/m_ibm.fpp (4 hunks)
  • tests/127A967A/golden-metadata.txt (1 hunks)
  • tests/127A967A/golden.txt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • examples/2D_mibm_cylinder_in_cross_flow/case.py
  • tests/127A967A/golden-metadata.txt
🧰 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 with m_<feature> prefix (e.g., m_transport)
Name public subroutines as s_<verb>_<noun> (e.g., s_compute_flux) and functions as f_<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
Avoid goto statements (except unavoidable legacy); avoid global state (COMMON, save)
Every variable must have intent(in|out|inout) specification and appropriate dimension / allocatable / pointer
Use s_mpi_abort(<msg>) for error termination instead of stop
Use !> style documentation for header comments; follow Doxygen Fortran format with !! @param and !! @return tags
Use implicit none statement in all modules
Use private declaration followed by explicit public exports in modules
Use derived types with pointers for encapsulation (e.g., pointer, dimension(:,:,:) => null())
Use pure and elemental attributes for side-effect-free functions; combine them for array ...

Files:

  • src/common/m_mpi_common.fpp
  • src/simulation/m_ibm.fpp
src/**/*.fpp

📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)

src/**/*.fpp: Use .fpp file extension for Fypp preprocessed files; CMake transpiles them to .f90
Start module files with Fypp include for macros: #:include 'macros.fpp'
Use the fypp ASSERT macro for validating conditions: @:ASSERT(predicate, message)
Use fypp macro @:ALLOCATE(var1, var2) for device-aware allocation instead of standard Fortran allocate
Use fypp macro @:DEALLOCATE(var1, var2) for device-aware deallocation instead of standard Fortran deallocate

Files:

  • src/common/m_mpi_common.fpp
  • 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 from src/common/include/parallel_macros.fpp instead
Wrap tight loops with $:GPU_PARALLEL_FOR(private='[...]', copy='[...]') macro; add collapse=n for safe nested loop merging
Declare loop-local variables with private='[...]' in GPU parallel loop macros
Allocate large arrays with managed or move them into a persistent $:GPU_ENTER_DATA(...) region at start-up
Do not place stop or error stop inside device code

Files:

  • src/simulation/m_ibm.fpp
🧠 Learnings (22)
📚 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.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.fpp
  • 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/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} : Declare loop-local variables with `private='[...]'` in GPU parallel loop macros

Applied to files:

  • src/common/m_mpi_common.fpp
  • 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/common/m_mpi_common.fpp
  • 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/common/m_mpi_common.fpp
  • 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} : Keep private helpers in the module; avoid nested procedures

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} : Every variable must have `intent(in|out|inout)` specification and appropriate `dimension` / `allocatable` / `pointer`

Applied to files:

  • src/common/m_mpi_common.fpp
  • 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/common/m_mpi_common.fpp
  • 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/common/m_mpi_common.fpp
  • 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 `s_mpi_abort(<msg>)` for error termination instead of `stop`

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} : Avoid `goto` statements (except unavoidable legacy); avoid global state (`COMMON`, `save`)

Applied to files:

  • src/common/m_mpi_common.fpp
  • 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/**/*.fpp : Use fypp macro `@:ALLOCATE(var1, var2)` for device-aware allocation instead of standard Fortran `allocate`

Applied to files:

  • src/common/m_mpi_common.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} : Call s_mpi_abort(<msg>) for errors, never use stop or error stop

Applied to files:

  • src/common/m_mpi_common.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} : Forbid goto statements (except in legacy code), COMMON blocks, and save globals

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/**/*.fpp : Use fypp macro `@:DEALLOCATE(var1, var2)` for device-aware deallocation instead of standard Fortran `deallocate`

Applied to files:

  • src/common/m_mpi_common.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} : 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 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: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} : 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 (1)
src/simulation/m_ibm.fpp (1)
toolchain/mfc/test/cases.py (1)
  • alter_ib (377-422)
🪛 GitHub Actions: Pretty
src/simulation/m_ibm.fpp

[error] 198-210: MFC formatting changed code. The formatting step exited with code 1 (diffs present). Run the formatting tool again or commit the changes. Command: './mfc.sh format -j $(nproc)'

⏰ 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). (14)
  • GitHub Check: Coverage Test on CodeCov
  • GitHub Check: Github (ubuntu, mpi, no-debug, true)
  • GitHub Check: Github (macos, mpi, debug, false)
  • GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
  • GitHub Check: Github (ubuntu, mpi, debug, false)
  • GitHub Check: Github (ubuntu, mpi, no-debug, false)
  • GitHub Check: Github (ubuntu, mpi, debug, true)
  • GitHub Check: Github (macos, mpi, no-debug, false)
  • GitHub Check: Self Hosted (gpu, acc, gt)
  • GitHub Check: Self Hosted (gpu, omp, gt)
  • GitHub Check: Self Hosted (cpu, none, gt)
  • GitHub Check: Code Cleanliness Check
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Build & Publish
🔇 Additional comments (3)
src/common/m_mpi_common.fpp (1)

475-499: LGTM on the overall implementation.

The new s_mpi_allreduce_vectors_sum subroutine correctly handles both in-place and separate buffer MPI reductions, with proper non-MPI fallback. The logic mirrors existing patterns in the module.

src/simulation/m_ibm.fpp (2)

1096-1144: LGTM on moment of inertia calculations.

The analytic formulas for circle (I = ½mr²), rectangle, and sphere (I = ⅖mr²) are correct. The numerical approximation branch properly accumulates contributions via dot_product(vector_to_axis, vector_to_axis). Previous issues with loop bounds and variable names have been addressed.


199-210: Unable to verify due to repository access failure. The review comment raises two specific concerns that require codebase examination:

  1. Formatting issue: Claims CI flagged formatting changes needed. Verification requires inspecting the actual file and checking git diff output.

  2. Logic scope mismatch: Suggests the comment "Moving IBM interior Pressure Values" is misleading because the condition ib_markers%sf(j, k, l) /= 0 applies to all IB interiors, not just moving ones. Verification requires understanding whether this code is within a moving IBM-specific subroutine context.

Without repository access, both claims cannot be confirmed or refuted. Manual verification by examining the file context around lines 199-210 and checking the CI logs is recommended.

patch_ib(i)%torque(:) = matmul(patch_ib(i)%rotation_matrix_inverse, torques(i, :)) ! torques must be computed in the local coordinates of the IB
end do
print *, forces(1, 1:2)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove debug print statement.

This debug output will produce excessive noise in production runs. Remove or guard with a debug flag.

-        print *, forces(1, 1:2)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
print *, forces(1, 1:2)
🤖 Prompt for AI Agents
In src/simulation/m_ibm.fpp around line 1064, there is a debug print statement
"print *, forces(1, 1:2)" that will produce excessive runtime output; remove
this line or wrap it in a conditional that checks a debug/logging flag (e.g., if
(debug) then ... end if) so it only prints when debugging is enabled, and ensure
any added flag is initialized/controlled by existing logging or
compilation/debug configuration.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 5 files (changes from recent commits).

Prompt for AI agents (all 2 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:1064">
P2: Debug print statement should be removed before merging. This will pollute output logs and may cause performance issues in parallel execution.</violation>
</file>

<file name="examples/2D_mibm_cylinder_in_cross_flow/case.py">

<violation number="1" location="examples/2D_mibm_cylinder_in_cross_flow/case.py:98">
P2: Misleading comment: The comment says `# z-axis rotation` but this parameter sets the mass of the immersed boundary, not rotation. This appears to be a copy-paste error from the previous line.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

patch_ib(i)%torque(:) = matmul(patch_ib(i)%rotation_matrix_inverse, torques(i, :)) ! torques must be computed in the local coordinates of the IB
end do
print *, forces(1, 1:2)
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 4, 2025

Choose a reason for hiding this comment

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

P2: Debug print statement should be removed before merging. This will pollute output logs and may cause performance issues in parallel execution.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_ibm.fpp, line 1064:

<comment>Debug print statement should be removed before merging. This will pollute output logs and may cause performance issues in parallel execution.</comment>

<file context>
@@ -1059,6 +1061,8 @@ contains
             patch_ib(i)%torque(:) = matmul(patch_ib(i)%rotation_matrix_inverse, torques(i, :)) ! torques must be computed in the local coordinates of the IB
         end do
 
+        print *, forces(1, 1:2)
+
     end subroutine s_compute_ib_forces
</file context>
Fix with Cubic

"patch_ib(1)%angular_vel(1)": 0.0, # x-axis rotational velocity in radians per second
"patch_ib(1)%angular_vel(2)": 0.0, # y-axis rotation
"patch_ib(1)%angular_vel(3)": 0.0, # z-axis rotation
"patch_ib(1)%mass": 1.0e-6, # z-axis rotation
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 4, 2025

Choose a reason for hiding this comment

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

P2: Misleading comment: The comment says # z-axis rotation but this parameter sets the mass of the immersed boundary, not rotation. This appears to be a copy-paste error from the previous line.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/2D_mibm_cylinder_in_cross_flow/case.py, line 98:

<comment>Misleading comment: The comment says `# z-axis rotation` but this parameter sets the mass of the immersed boundary, not rotation. This appears to be a copy-paste error from the previous line.</comment>

<file context>
@@ -95,7 +95,7 @@
             &quot;patch_ib(1)%angular_vel(2)&quot;: 0.0,  # y-axis rotation
             &quot;patch_ib(1)%angular_vel(3)&quot;: 0.0,  # z-axis rotation
-            &quot;patch_ib(1)%mass&quot;: 0.0001,  # z-axis rotation
+            &quot;patch_ib(1)%mass&quot;: 1.0e-6,  # z-axis rotation
             # Fluids Physical Parameters
             &quot;fluid_pp(1)%gamma&quot;: 1.0e00 / (gam_a - 1.0e00),  # 2.50(Not 1.40)
</file context>
Suggested change
"patch_ib(1)%mass": 1.0e-6, # z-axis rotation
"patch_ib(1)%mass": 1.0e-6, # mass of the immersed boundary
Fix with Cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review effort 4/5 size:XL This PR changes 500-999 lines, ignoring generated files

Development

Successfully merging this pull request may close these issues.

1 participant