Skip to content

Replace fprettify with ffmt Fortran formatter#1334

Open
sbryngelson wants to merge 3 commits intoMFlowCode:masterfrom
sbryngelson:packages
Open

Replace fprettify with ffmt Fortran formatter#1334
sbryngelson wants to merge 3 commits intoMFlowCode:masterfrom
sbryngelson:packages

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Mar 21, 2026

Summary

Replace fprettify + indenter.py with ffmt, a fast, configurable Fortran formatter written in Rust.

Why

  • fprettify is unmaintained (last release 2020)
  • fprettify requires 2-4 passes to converge (not idempotent)
  • fprettify has no OpenACC/OpenMP directive awareness (needed separate indenter.py)
  • fprettify has fragile Fypp support

What changes

Toolchain:

  • toolchain/bootstrap/format.sh — single ffmt call replaces multi-pass fprettify + indenter.py loop
  • toolchain/pyproject.tomlfprettifyffmt
  • ffmt.toml — MFC formatting configuration
  • Remove toolchain/indenter.py

Source formatting (all .fpp/.f90 files):

  • Operator spacing standardized
  • Keywords lowercased, compound keywords split (enddoend do)
  • Named end statements (end subroutineend subroutine s_foo)
  • Declaration :: alignment
  • Comment wrapping at 132 chars with Doxygen !> / !! support
  • Unicode symbols → LaTeX (σ\sigma, \partial)
  • Fypp macro spacing (@:ALLOCATE (@:ALLOCATE()
  • Space after ! in comments, keyword-paren spacing (if(if ()

Install

pip install ffmt

Links

Test plan

  • ./mfc.sh format -j 8 works with ffmt
  • ./mfc.sh precheck -j 8 passes
  • ./mfc.sh build -j 8 compiles
  • ./mfc.sh test -j 8 passes
  • Formatting is idempotent (ffmt --check src/ returns 0)

@sbryngelson sbryngelson force-pushed the packages branch 3 times, most recently from 56d5339 to 4bbff8d Compare March 21, 2026 01:48
@sbryngelson sbryngelson marked this pull request as ready for review March 21, 2026 03:05
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 21, 2026

📝 Walkthrough

Walkthrough

This pull request applies code formatting standardization across the codebase. Changes include consolidating array dimension spacing from (:, :) to (:,:), normalizing control-flow keywords from elseif to else if, lowercasing intent declarations, adjusting whitespace around string concatenation operators, reformatting GPU macro directives, and aligning variable declarations. A new ffmt.toml configuration file establishes formatting defaults, and .gitignore is updated to exclude the .ffmt_cache/ directory. These modifications affect formatting, indentation, and layout without altering logic or computation.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: replacing fprettify with ffmt as the Fortran formatter.
Description check ✅ Passed The PR description is comprehensive, covering motivation, technical changes, installation, and testing. However, the template requires explicit sections (Type of change, Testing, Checklist) which are not formally separated in the provided description.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

Tip

You can customize the tone of the review comments and chat replies.

Configure the tone_instructions setting to customize the tone of the review comments and chat replies. For example, you can set the tone to Act like a strict teacher, Act like a pirate and more.

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

Caution

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

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

1557-1590: ⚠️ Potential issue | 🟠 Major

Sync device state before building the restart buffer.

This routine packs gas_p, gas_mv, intfc_rad, intfc_vel, Rmax_stats, and related fields on the host, but those values are updated in GPU kernels elsewhere in the module. Without a GPU_UPDATE(host=...) here, non-unified-memory runs can write stale restart data.

💡 Suggested fix
+        $:GPU_UPDATE(host='[lag_id, mtn_pos, mtn_posPrev, mtn_vel, intfc_rad, intfc_vel, bub_R0, Rmax_stats, Rmin_stats, &
+            & bub_dphidt, gas_p, gas_mv, gas_mg, gas_betaT, gas_betaC]')
+
         if (bub_id > 0) then
             allocate (MPI_IO_DATA_lag_bubbles(max(1, bub_id), 1:lag_io_vars))
Based on learnings: Use GPU_UPDATE macro with FYPP eval directive ($:) to synchronize data between CPU and GPU with parameters: host (GPU to CPU), device (CPU to GPU).
src/common/m_mpi_common.fpp (1)

273-286: ⚠️ Potential issue | 🔴 Critical

Guard the receive-side allocations to the root rank and provide a serial fallback.

MPI_GATHER populates recounts only on root. Non-root ranks then use uninitialized recounts values to compute displs and allocate gathered_vector, causing undefined behavior. Additionally, the non-MPI build has no #else clause, leaving the intent(out) gathered_vector unallocated despite the subroutine's contract requiring it to be assigned.

Suggested fix
 `#ifdef` MFC_MPI
-        allocate (recounts(num_procs))
+        allocate (recounts(num_procs), displs(num_procs))
+        recounts = 0
+        displs = 0
 
         call MPI_GATHER(counts, 1, MPI_INTEGER, recounts, 1, MPI_INTEGER, root, MPI_COMM_WORLD, ierr)
-
-        allocate (displs(size(recounts)))
-
-        displs(1) = 0
-
-        do i = 2, size(recounts)
-            displs(i) = displs(i - 1) + recounts(i - 1)
-        end do
-
-        allocate (gathered_vector(sum(recounts)))
+        if (proc_rank == root) then
+            do i = 2, size(recounts)
+                displs(i) = displs(i - 1) + recounts(i - 1)
+            end do
+            allocate (gathered_vector(sum(recounts)))
+        else
+            allocate (gathered_vector(0))
+        end if
         call MPI_GATHERV(my_vector, counts, mpi_p, gathered_vector, recounts, displs, mpi_p, root, MPI_COMM_WORLD, ierr)
+#else
+        allocate (gathered_vector(counts))
+        gathered_vector = my_vector
 `#endif`
🧹 Nitpick comments (3)
src/common/m_compile_specific.f90 (1)

15-96: Please run full three-target tests for this src/common change.

Even formatting-only updates here affect all executables, so run the complete matrix (./mfc.sh test -j 8).

Based on learnings: Changes to src/common/ affect all three executables (pre_process, simulation, post_process) and should be tested comprehensively.

src/common/m_variables_conversion.fpp (1)

353-355: Follow through on the GPU TODOs before they become stale debt.

The TODOs in Line 353 and Line 354 sit on a hot-path routine and are easy to lose during formatter-only PRs. Please either resolve or open a tracked issue and reference it inline.

If you want, I can draft the issue text (scope + acceptance criteria) for these two TODOs.

src/pre_process/m_check_patches.fpp (1)

529-531: Make s_check_model_geometry self-contained for patch-id formatting.

This message currently depends on module-global iStr being set by callers. Converting patch_id locally in this subroutine removes hidden coupling.

♻️ Proposed refactor
 impure subroutine s_check_model_geometry(patch_id)
 
     integer, intent(in) :: patch_id
 
+    character(len=10) :: patch_id_str
     logical :: file_exists
 
+    call s_int_to_str(patch_id, patch_id_str)
     inquire (file=patch_icpp(patch_id)%model_filepath, exist=file_exists)
 
     @:PROHIBIT(.not. file_exists, &
         & "Model file " // trim(patch_icpp(patch_id)%model_filepath) // " requested by patch " // trim(iStr) &
         & // " does not exist")
! Replace trim(iStr) with:
trim(patch_id_str)

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c0861c00-8165-4045-9914-ad497a3bd230

📥 Commits

Reviewing files that changed from the base of the PR and between 90d4b6e and 4bbff8d.

📒 Files selected for processing (84)
  • .gitignore
  • ffmt.toml
  • src/common/include/1dHardcodedIC.fpp
  • src/common/include/2dHardcodedIC.fpp
  • src/common/include/3dHardcodedIC.fpp
  • src/common/include/ExtrusionHardcodedIC.fpp
  • src/common/include/acc_macros.fpp
  • src/common/include/macros.fpp
  • src/common/include/omp_macros.fpp
  • src/common/m_boundary_common.fpp
  • src/common/m_checker_common.fpp
  • src/common/m_chemistry.fpp
  • src/common/m_compile_specific.f90
  • src/common/m_constants.fpp
  • src/common/m_delay_file_access.f90
  • src/common/m_derived_types.fpp
  • src/common/m_finite_differences.fpp
  • src/common/m_helper.fpp
  • src/common/m_helper_basic.fpp
  • src/common/m_model.fpp
  • src/common/m_mpi_common.fpp
  • src/common/m_nvtx.f90
  • src/common/m_phase_change.fpp
  • src/common/m_precision_select.f90
  • src/common/m_variables_conversion.fpp
  • src/post_process/m_checker.fpp
  • src/post_process/m_data_input.f90
  • src/post_process/m_data_output.fpp
  • src/post_process/m_derived_variables.fpp
  • src/post_process/m_global_parameters.fpp
  • src/post_process/m_mpi_proxy.fpp
  • src/post_process/m_start_up.fpp
  • src/post_process/p_main.fpp
  • src/pre_process/m_assign_variables.fpp
  • src/pre_process/m_boundary_conditions.fpp
  • src/pre_process/m_check_ib_patches.fpp
  • src/pre_process/m_check_patches.fpp
  • src/pre_process/m_checker.fpp
  • src/pre_process/m_data_output.fpp
  • src/pre_process/m_global_parameters.fpp
  • src/pre_process/m_grid.f90
  • src/pre_process/m_icpp_patches.fpp
  • src/pre_process/m_initial_condition.fpp
  • src/pre_process/m_mpi_proxy.fpp
  • src/pre_process/m_perturbation.fpp
  • src/pre_process/m_simplex_noise.fpp
  • src/pre_process/m_start_up.fpp
  • src/pre_process/p_main.f90
  • src/simulation/include/inline_riemann.fpp
  • src/simulation/m_acoustic_src.fpp
  • src/simulation/m_body_forces.fpp
  • src/simulation/m_bubbles.fpp
  • src/simulation/m_bubbles_EE.fpp
  • src/simulation/m_bubbles_EL.fpp
  • src/simulation/m_bubbles_EL_kernels.fpp
  • src/simulation/m_cbc.fpp
  • src/simulation/m_checker.fpp
  • src/simulation/m_compute_cbc.fpp
  • src/simulation/m_compute_levelset.fpp
  • src/simulation/m_data_output.fpp
  • src/simulation/m_derived_variables.fpp
  • src/simulation/m_fftw.fpp
  • src/simulation/m_global_parameters.fpp
  • src/simulation/m_hyperelastic.fpp
  • src/simulation/m_hypoelastic.fpp
  • src/simulation/m_ib_patches.fpp
  • src/simulation/m_ibm.fpp
  • src/simulation/m_igr.fpp
  • src/simulation/m_mpi_proxy.fpp
  • src/simulation/m_muscl.fpp
  • src/simulation/m_pressure_relaxation.fpp
  • src/simulation/m_qbmm.fpp
  • src/simulation/m_rhs.fpp
  • src/simulation/m_riemann_solvers.fpp
  • src/simulation/m_sim_helpers.fpp
  • src/simulation/m_start_up.fpp
  • src/simulation/m_surface_tension.fpp
  • src/simulation/m_time_steppers.fpp
  • src/simulation/m_viscous.fpp
  • src/simulation/m_weno.fpp
  • src/simulation/p_main.fpp
  • toolchain/bootstrap/format.sh
  • toolchain/indenter.py
  • toolchain/pyproject.toml

# MFC Fortran formatting configuration
# These are the defaults — this file makes them explicit.

indent-width = 4
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

Use 2-space indentation in formatter config.

Line 4 sets indent-width = 4, which conflicts with the project Fortran style and can cause persistent formatting drift.

Suggested fix
-indent-width = 4
+indent-width = 2

As per coding guidelines: Use 2-space indentation, lowercase keywords, and explicit intent on all subroutine/function arguments in Fortran code.

📝 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
indent-width = 4
indent-width = 2

x_step = x_cc(1) - x_cc(0)
delta_x = merge(x_cc(0) - domain_xstart + x_step/2.0, &
x_cc(index_x) - domain_xstart + x_step/2.0, num_dims == 1)
delta_x = merge(x_cc(0) - domain_xstart + x_step/2.0, x_cc(index_x) - domain_xstart + x_step/2.0, num_dims == 1)
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

Use _wp-qualified literals in delta_x expression.

2.0 should be 2.0_wp to keep kind-consistent arithmetic with real(wp) operands.

Proposed fix
-            delta_x = merge(x_cc(0) - domain_xstart + x_step/2.0, x_cc(index_x) - domain_xstart + x_step/2.0, num_dims == 1)
+            delta_x = merge(x_cc(0) - domain_xstart + x_step/2.0_wp, x_cc(index_x) - domain_xstart + x_step/2.0_wp, num_dims == 1)

As per coding guidelines: “Use real(wp) for all computational variables; literal constants need the _wp suffix (e.g., 1.0_wp, 1e-6_wp).”


#ifdef _WIN32
call system('for /F %i in ("'//trim(dirpath)//'") do @echo %~ni > '//trim(tmpfilepath))
call system('for /F %i in ("' // trim(dirpath) // '") do @echo %~ni > ' // trim(tmpfilepath))
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

Shorten Line 85 to pass style checks.

Line 85 exceeds the 100-char limit (S001).

Suggested fix
-        call system('for /F %i in ("' // trim(dirpath) // '") do `@echo` %~ni > ' // trim(tmpfilepath))
+        call system('for /F %i in ("' // trim(dirpath) // '") do `@echo` %~ni > ' // &
+            trim(tmpfilepath))
📝 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
call system('for /F %i in ("' // trim(dirpath) // '") do @echo %~ni > ' // trim(tmpfilepath))
call system('for /F %i in ("' // trim(dirpath) // '") do `@echo` %~ni > ' // &
trim(tmpfilepath))
🧰 Tools
🪛 Fortitude (0.8.0)

[error] 85-85: line length of 101, exceeds maximum 100

(S001)

Comment on lines 372 to 383
impure subroutine s_mpi_allreduce_sum(var_loc, var_glb)

real(wp), intent(in) :: var_loc
real(wp), intent(in) :: var_loc
real(wp), intent(out) :: var_glb

#ifdef MFC_MPI
integer :: ierr !< Generic flag used to identify and report MPI errors

! Performing the reduction procedure
call MPI_ALLREDUCE(var_loc, var_glb, 1, mpi_p, &
MPI_SUM, MPI_COMM_WORLD, ierr)
call MPI_ALLREDUCE(var_loc, var_glb, 1, mpi_p, MPI_SUM, MPI_COMM_WORLD, ierr)

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

Return the local value when MPI is disabled.

These wrappers leave var_glb undefined in non-MPI builds, unlike s_mpi_allreduce_integer_sum and s_mpi_allreduce_vectors_sum. The serial path should preserve the caller contract by copying var_loc.

💡 Suggested fix
 impure subroutine s_mpi_allreduce_sum(var_loc, var_glb)
 ...
 `#ifdef` MFC_MPI
         call MPI_ALLREDUCE(var_loc, var_glb, 1, mpi_p, MPI_SUM, MPI_COMM_WORLD, ierr)
+#else
+        var_glb = var_loc
 `#endif`
 ...
 impure subroutine s_mpi_allreduce_min(var_loc, var_glb)
 ...
 `#ifdef` MFC_MPI
         call MPI_ALLREDUCE(var_loc, var_glb, 1, mpi_p, MPI_MIN, MPI_COMM_WORLD, ierr)
+#else
+        var_glb = var_loc
 `#endif`
 ...
 impure subroutine s_mpi_allreduce_max(var_loc, var_glb)
 ...
 `#ifdef` MFC_MPI
         call MPI_ALLREDUCE(var_loc, var_glb, 1, mpi_p, MPI_MAX, MPI_COMM_WORLD, ierr)
+#else
+        var_glb = var_loc
 `#endif`

Also applies to: 437-448, 457-468

Comment on lines +12 to +13
integer, private :: col(7) = [int(Z'0000ff00'), int(Z'000000ff'), int(Z'00ffff00'), int(Z'00ff00ff'), int(Z'0000ffff'), &
& int(Z'00ff0000'), int(Z'00ffffff')]
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

Wrap the col initializer to satisfy lint line-length limits.

Line 12 currently exceeds the configured maximum length (Fortitude S001).

🧹 Proposed fix
-    integer, private :: col(7) = [int(Z'0000ff00'), int(Z'000000ff'), int(Z'00ffff00'), int(Z'00ff00ff'), int(Z'0000ffff'), &
-        & int(Z'00ff0000'), int(Z'00ffffff')]
+    integer, private :: col(7) = [ &
+        int(Z'0000ff00'), int(Z'000000ff'), int(Z'00ffff00'), &
+        int(Z'00ff00ff'), int(Z'0000ffff'), int(Z'00ff0000'), &
+        int(Z'00ffffff') &
+    ]
📝 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
integer, private :: col(7) = [int(Z'0000ff00'), int(Z'000000ff'), int(Z'00ffff00'), int(Z'00ff00ff'), int(Z'0000ffff'), &
& int(Z'00ff0000'), int(Z'00ffffff')]
integer, private :: col(7) = [ &
int(Z'0000ff00'), int(Z'000000ff'), int(Z'00ffff00'), &
int(Z'00ff00ff'), int(Z'0000ffff'), int(Z'00ff0000'), &
int(Z'00ffffff') &
]
🧰 Tools
🪛 Fortitude (0.8.0)

[error] 12-12: line length of 125, exceeds maximum 100

(S001)

Comment on lines +310 to 311
q_prim_vf(i)%sf = q_prim_vf(i)%sf*(1._wp - q_prim_vf(alf_idx)%sf)/alf_sum%sf
end do
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 | 🔴 Critical

Use cell-indexed updates here; current assignments are array-wide.

In Line 310 (and repeated at Line 335, Line 375, Line 487, Line 609), q_prim_vf(i)%sf and alf_sum%sf are updated without (j,k,l). Inside this per-cell routine, that writes full fields instead of the current cell and can corrupt neighboring cells.

Proposed fix pattern (apply to each repeated block)
-            alf_sum%sf = 0._wp
+            alf_sum%sf(j, k, l) = 0._wp
             do i = adv_idx%beg, adv_idx%end - 1
-                alf_sum%sf = alf_sum%sf + q_prim_vf(i)%sf
+                alf_sum%sf(j, k, l) = alf_sum%sf(j, k, l) + q_prim_vf(i)%sf(j, k, l)
             end do

             do i = adv_idx%beg, adv_idx%end - 1
-                q_prim_vf(i)%sf = q_prim_vf(i)%sf*(1._wp - q_prim_vf(alf_idx)%sf)/alf_sum%sf
+                q_prim_vf(i)%sf(j, k, l) = q_prim_vf(i)%sf(j, k, l) * &
+                    (1._wp - q_prim_vf(alf_idx)%sf(j, k, l)) / alf_sum%sf(j, k, l)
             end do

Also applies to: 335-336, 375-376, 487-488, 609-610

Comment on lines +1533 to +1535
if (proc_rank == 0 .and. mod(cell_num, ncells/100) == 0) then
write (*, "(A, I3, A)", advance="no") char(13) // " * Generating grid: ", nint(100*real(cell_num)/ncells), "%"
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

Guard progress modulus against zero stride.

ncells/100 can evaluate to zero on small meshes, making mod(cell_num, 0) invalid at runtime.

🛠️ Proposed fix
-            if (proc_rank == 0 .and. mod(cell_num, ncells/100) == 0) then
+            if (proc_rank == 0 .and. mod(cell_num, max(1, ncells/100)) == 0) then
                 write (*, "(A, I3, A)", advance="no") char(13) // "  * Generating grid: ", nint(100*real(cell_num)/ncells), "%"
             end if
📝 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
if (proc_rank == 0 .and. mod(cell_num, ncells/100) == 0) then
write (*, "(A, I3, A)", advance="no") char(13) // " * Generating grid: ", nint(100*real(cell_num)/ncells), "%"
end if
if (proc_rank == 0 .and. mod(cell_num, max(1, ncells/100)) == 0) then
write (*, "(A, I3, A)", advance="no") char(13) // " * Generating grid: ", nint(100*real(cell_num)/ncells), "%"
end if


!> @brief This program takes care of setting up the initial condition and
!! grid data for the multicomponent flow code.
!> @brief This program takes care of setting up the initial condition and grid data for the multicomponent flow code.
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

Wrap Line 5 to satisfy lint line-length.

Line 5 is 130 chars and triggers Fortitude S001 (max 100), which can fail checks.

Suggested fix
-!> `@brief` This program takes care of setting up the initial condition and              grid data for the multicomponent flow code.
+!> `@brief` This program takes care of setting up the initial condition and grid data
+!! for the multicomponent flow code.
📝 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
!> @brief This program takes care of setting up the initial condition and grid data for the multicomponent flow code.
!> `@brief` This program takes care of setting up the initial condition and grid data
!! for the multicomponent flow code.
🧰 Tools
🪛 Fortitude (0.8.0)

[error] 5-5: line length of 130, exceeds maximum 100

(S001)

real(wp) :: nR3bar
integer(wp) :: i, j, k, l
real(wp) :: nR3bar
integer(wp) :: i, j, k, l
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if integer(wp) was already present or introduced by the formatter
# Search for similar declarations in the codebase
rg -n "integer\(wp\)" --type-add 'fortran:*.{fpp,f90}' --type fortran | head -30

Repository: MFlowCode/MFC

Length of output: 168


🏁 Script executed:

#!/bin/bash
# Check loop variable declarations in the same file
echo "=== Loop declarations in m_bubbles_EE.fpp ==="
rg -n "integer.*::" src/simulation/m_bubbles_EE.fpp | head -20

echo ""
echo "=== Typical loop index declarations across codebase ==="
rg -n "integer\s+::\s+[i|j|k|l]" --type-add 'fortran:*.{fpp,f90}' --type fortran | head -15

echo ""
echo "=== Check for wp definition in the module ==="
rg -n "parameter.*wp|wp\s*=" src/simulation/m_bubbles_EE.fpp | head -10

Repository: MFlowCode/MFC

Length of output: 2039


Incorrect loop index type declaration at line 77.

Loop iterators i, j, k, l should be declared as integer, not integer(wp). The wp parameter represents floating-point working precision and should never qualify integer types. Correct to:

integer :: i, j, k, l

Comment on lines 229 to 232
! Create ./D directory
write (path_D_dir, '(A,I0,A,I0)') trim(case_dir)//'/D'
write (path_D_dir, '(A,I0,A,I0)') trim(case_dir) // '/D'
call my_inquire(path_D_dir, file_exist)
if (.not. file_exist) call s_create_directory(trim(path_D_dir))
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 | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

head -n 232 src/simulation/m_bubbles_EL.fpp | tail -n 15

Repository: MFlowCode/MFC

Length of output: 642


🏁 Script executed:

rg -n "path_D_dir" src/simulation/m_bubbles_EL.fpp -B 2 -A 2

Repository: MFlowCode/MFC

Length of output: 661


Replace the internal write with a direct assignment.

The format string '(A,I0,A,I0)' expects four items (A, I0, A, I0), but only one character expression is provided. This causes a runtime error on the fresh-start path when save_count == 0.

Suggested fix
-            write (path_D_dir, '(A,I0,A,I0)') trim(case_dir) // '/D'
+            path_D_dir = trim(case_dir) // '/D'
📝 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
! Create ./D directory
write (path_D_dir, '(A,I0,A,I0)') trim(case_dir)//'/D'
write (path_D_dir, '(A,I0,A,I0)') trim(case_dir) // '/D'
call my_inquire(path_D_dir, file_exist)
if (.not. file_exist) call s_create_directory(trim(path_D_dir))
! Create ./D directory
path_D_dir = trim(case_dir) // '/D'
call my_inquire(path_D_dir, file_exist)
if (.not. file_exist) call s_create_directory(trim(path_D_dir))

@sbryngelson sbryngelson force-pushed the packages branch 3 times, most recently from d6a4128 to c4f7661 Compare March 21, 2026 04:05
Replace fprettify + indenter.py with ffmt (https://github.com/sbryngelson/ffmt),
a fast, configurable Fortran formatter written in Rust.

Integration:
- toolchain/bootstrap/format.sh: single ffmt call replaces multi-pass
  fprettify + indenter.py loop
- toolchain/pyproject.toml: replace fprettify dependency with ffmt
- ffmt.toml: MFC formatting configuration
- .gitignore: add .ffmt_cache/
- Remove toolchain/indenter.py

Source formatting:
- All .fpp/.f90 files reformatted with ffmt v0.2.0
- Unicode symbols replaced with LaTeX (sigma, pi, partial, etc.)
- Operator spacing, keyword casing, declaration alignment
- Named end statements, comment wrapping at 132 chars
- Doxygen comment block re-wrapping

Install: pip install ffmt
Repo: https://github.com/sbryngelson/ffmt
@codecov
Copy link

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 37.37575% with 630 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.50%. Comparing base (febffab) to head (cb33f11).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/post_process/m_data_output.fpp 30.76% 95 Missing and 4 partials ⚠️
src/common/m_mpi_common.fpp 31.14% 72 Missing and 12 partials ⚠️
src/common/m_boundary_common.fpp 48.75% 81 Missing and 1 partial ⚠️
src/post_process/m_derived_variables.fpp 9.37% 58 Missing ⚠️
src/post_process/m_start_up.fpp 23.21% 40 Missing and 3 partials ⚠️
src/post_process/m_mpi_proxy.fpp 4.54% 42 Missing ⚠️
src/common/m_model.fpp 37.93% 36 Missing ⚠️
src/post_process/m_data_input.f90 20.68% 23 Missing ⚠️
src/common/m_variables_conversion.fpp 59.25% 22 Missing ⚠️
src/pre_process/m_check_patches.fpp 4.54% 21 Missing ⚠️
... and 13 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1334      +/-   ##
==========================================
+ Coverage   45.01%   47.50%   +2.49%     
==========================================
  Files          70       70              
  Lines       20562    19152    -1410     
  Branches     1962     1631     -331     
==========================================
- Hits         9255     9098     -157     
+ Misses      10179     9182     -997     
+ Partials     1128      872     -256     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant