Fix WP_MOK hardcoded to 8 bytes, use storage_size for portability

[sbryngelson]

User description

Summary

Severity: HIGH — MPI file I/O reads from wrong offsets in single-precision builds.

WP_MOK (the MPI offset for the working-precision real size) is hardcoded to int(8._wp, MPI_OFFSET_KIND), assuming 8-byte (double-precision) reals. In single-precision builds where wp corresponds to 4 bytes, this causes all MPI file read offsets to be wrong by a factor of 2.

Before

WP_MOK = int(8._wp, MPI_OFFSET_KIND)    ! assumes double precision

After

WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)    ! actual byte size

All 10 occurrences across 5 files are fixed. One site in src/simulation/m_start_up.fpp had int(4._wp, ...) instead of int(8._wp, ...) — wrong in both single and double precision.

Additionally, 3 sites in pre_process used storage_size(0._wp) instead of storage_size(0._stp) — since field arrays use storage precision (stp), the I/O byte stride must match stp, not wp.

Why this went undetected

The codebase is predominantly used in double precision where wp = stp = 8 bytes, so the hardcoded value happens to be correct.

Also fixes: dq_prim_dy_vf → dq_prim_dz_vf bug in 2D cylindrical viscous boundary

In src/simulation/m_rhs.fpp, the call to s_compute_viscous_stress_cylindrical_boundary in the p == 0 (2D) branch was passing dq_prim_dy_vf as the z-gradient argument (grad_z_vf) instead of dq_prim_dz_vf. The p > 0 (3D) branch was correct. This was a duplicate-line copy-paste error that would cause incorrect viscous stress computation at cylindrical boundaries in 2D simulations.

Files changed

• src/post_process/m_data_input.f90 — 3 WP_MOK sites
• src/pre_process/m_data_output.fpp — 2 WP_MOK sites
• src/pre_process/m_start_up.fpp — 1 WP_MOK site
• src/simulation/m_data_output.fpp — 3 WP_MOK sites
• src/simulation/m_start_up.fpp — 1 WP_MOK site
• src/simulation/m_rhs.fpp — 1 dq_prim_dy_vf → dq_prim_dz_vf fix

Test plan

• Build in both single and double precision
• Verify MPI parallel I/O reads correct data in both modes
• Verify 2D cylindrical viscous boundary cases produce correct results

🤖 Generated with Claude Code

Fixes #1207

[codeant-ai]

Nitpicks 🔍

🔒 No security issues identified

⚡ Recommended areas for review storage_size semantics The code now uses storage_size(0._wp)/8 to compute bytes per working precision real. Verify that storage_size returns bits on all supported compilers and that dividing by 8 yields the expected bytes (e.g. 4 or 8). Confirm this produces an integer and that no unintended real-to-integer truncation/rounding can occur in target builds. Inconsistent integer kinds In s_read_ib_data_files WP_MOK (and related MPI-offset variables) are declared as default INTEGER, but the assignment uses INT(..., MPI_OFFSET_KIND). This can cause implicit kind conversions and subtle bugs when offsets are used with MPI calls. Ensure local declarations use KIND=MPI_OFFSET_KIND where appropriate.

[Copilot]

Pull request overview

This PR fixes MPI file offset calculations that were hardcoded for 8-byte wp reals, making parallel I/O portable across single- and double-precision builds.

Changes:

• Replaced WP_MOK = int(8._wp, MPI_OFFSET_KIND) with a storage_size-based byte-size computation.
• Applied the fix across multiple MPI I/O read/setup paths to keep offsets consistent.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.04%. Comparing base (ab5082e) to head (43bbd2b).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1187   +/-   ##
=======================================
  Coverage   44.04%   44.04%           
=======================================
  Files          70       70           
  Lines       20499    20499           
  Branches     1993     1993           
=======================================
  Hits         9028     9028           
  Misses      10330    10330           
  Partials     1141     1141           

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

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (3)

src/simulation/m_data_output.fpp (2)

962-970: ⚠️ Potential issue | 🟠 Major

Same wp vs. stp mismatch as in the simulation read path — WP_MOK should use stp size for the shared-file write branch.

The disp byte-offset formula at lines 977/990/1003/1017:

disp = m_MOK*max(MOK, n_MOK)*max(MOK, p_MOK)*WP_MOK*(var_MOK - 1)

uses WP_MOK = storage_size(0._wp)/8. The actual write (MPI_FILE_WRITE_ALL) uses mpi_io_p (stp), meaning elements are real(stp) on disk. In a mixed-precision build the offset computed with wp size diverges from the actual stp element size, misaligning every variable's data block.

Note: MPI_FILE_SET_VIEW here uses mpi_p (wp) as etype while the write uses mpi_io_p (stp) — this inconsistency between etype and data type is a separate pre-existing issue, but the disp value itself must reflect the on-disk stp element size.

🔧 Proposed fix

-           WP_MOK = int(storage_size(0._wp)/8, MPI_OFFSET_KIND)
+           WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)

Based on learnings: "Ensure mpi_p matches wp and mpi_io_p matches stp" and "In mixed-precision mode: do not mix stp (storage) and wp (working) precision without intentional conversion."

🤖 Prompt for AI Agents

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

In `@src/simulation/m_data_output.fpp` around lines 962 - 970, The computed byte
stride WP_MOK is using working precision (wp) but the file writes use storage
precision (stp); change WP_MOK initialization to use storage_size(0._stp)/8 so
disk offsets match the on-disk element size (i.e., replace WP_MOK =
int(storage_size(0._wp)/8, MPI_OFFSET_KIND) with WP_MOK =
int(storage_size(0._stp)/8, MPI_OFFSET_KIND)), and verify the disp calculation
(disp = m_MOK*max(MOK, n_MOK)*max(MOK, p_MOK)*WP_MOK*(var_MOK - 1)) uses that
WP_MOK so offsets align with MPI_FILE_WRITE_ALL which uses mpi_io_p (stp).

---

1085-1105: ⚠️ Potential issue | 🟠 Major

s_write_parallel_ib_data: WP_MOK (real byte size) is incorrectly used to compute the displacement for integer IB marker data, and the sys_size-based base offset is spurious for a dedicated ib.dat file.

Two distinct issues:

1. Wrong element size. WP_MOK = storage_size(0._wp)/8 gives the byte count of a real(wp) value, but the IB data is written with MPI_INTEGER. In a double-precision build (WP_MOK = 8) this doubles the displacement relative to the actual 4-byte integer elements, placing subsequent IB snapshots at wrong offsets in the file. (In single-precision WP_MOK = 4, matching MPI_INTEGER size, so it accidentally works.)

2. Spurious sys_size base offset. var_MOK = sys_size + 1 was designed for a layout where IB data follows sys_size real-variable blocks. Since s_write_parallel_ib_data opens its own dedicated ib.dat file, there are no preceding real-variable blocks to skip; the sys_size term introduces an incorrectly large initial offset for all non-zero time steps.

The corrected formula should be:

disp = m_MOK*max(MOK, n_MOK)*max(MOK, p_MOK)*IB_MOK*int(time_step/t_step_save, MPI_OFFSET_KIND)

where IB_MOK holds the byte size of MPI_INTEGER (e.g. int(4, MPI_OFFSET_KIND) for standard builds, or int(1, MPI_OFFSET_KIND) under MFC_MIXED_PRECISION).

These are pre-existing issues not introduced by this PR, but the PR surfaces them by making the element-size derivation explicit.

🤖 Prompt for AI Agents

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

In `@src/simulation/m_data_output.fpp` around lines 1085 - 1105, The displacement
calculation in s_write_parallel_ib_data uses WP_MOK (storage_size of real) and a
sys_size-based var_MOK base offset, which is wrong for an ib.dat file of
MPI_INTEGERs; replace WP_MOK/var_MOK with an IB_MOK that represents the byte
size of MPI_INTEGER (set IB_MOK = int(4, MPI_OFFSET_KIND) normally or the
appropriate size under MFC_MIXED_PRECISION) and compute disp as
m_MOK*max(MOK,n_MOK)*max(MOK,p_MOK)*IB_MOK*int(time_step/t_step_save,
MPI_OFFSET_KIND), removing the sys_size (+1) term and the special-case
time_step==0 handling so the initial snapshot is at offset 0 and later snapshots
are spaced by integer-sized blocks.

src/simulation/m_start_up.fpp (1)

655-670: ⚠️ Potential issue | 🟠 Major

Fix WP_MOK to use storage precision size instead of working precision.

In mixed-precision builds (wp=real64, stp=real32), WP_MOK should be calculated from stp size, not wp size. The variable mpi_io_p correctly corresponds to stp (via MPI_BYTE when stp == half_precision), so file elements are written as 2 bytes each. However, WP_MOK = int(storage_size(0._wp)/8, ...) yields 8 bytes, causing the offset formula to calculate byte displacements at 4× the actual element size. This corrupts all MPI I/O reads after the first variable.

This issue exists across multiple files:

• src/simulation/m_start_up.fpp (lines 578, 655)
• src/pre_process/m_start_up.fpp (line 663)
• src/pre_process/m_data_output.fpp (lines 552, 618)
• src/simulation/m_data_output.fpp (lines 898, 966, 1090)
• src/post_process/m_data_input.f90 (lines 136, 180, 520)

Change storage_size(0._wp) to storage_size(0._stp) in all assignments.

🤖 Prompt for AI Agents

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

In `@src/simulation/m_start_up.fpp` around lines 655 - 670, The byte-offset WP_MOK
is being computed using working precision (storage_size(0._wp)) which is wrong
for mixed-precision I/O; change the WP_MOK assignments to use storage precision
instead (storage_size(0._stp)) so WP_MOK matches the actual element size used by
mpi_io_p/mpi_io_type; update every occurrence that sets WP_MOK (e.g., the WP_MOK
assignments in m_start_up.fpp, m_data_output.fpp and m_data_input.f90) so the
offset calculation that uses WP_MOK, MOK, n_MOK, p_MOK, var_MOK/dsp produces
correct byte displacements.

🧹 Nitpick comments (2)

src/simulation/m_start_up.fpp (1)

574-581: WP_MOK (and other MOK variants) are computed but never consumed in the file_per_process branch.

In this branch the reads are plain sequential MPI_FILE_READ calls with no MPI_FILE_SET_VIEW, so WP_MOK, MOK, str_MOK, and NVARS_MOK (lines 578–581) are dead. Likewise, the var_MOK assignments inside each loop body are also never used. These are copy-paste artefacts from the shared-file path.

🧹 Suggested cleanup

-               WP_MOK = int(storage_size(0._wp)/8, MPI_OFFSET_KIND)
-               MOK = int(1._wp, MPI_OFFSET_KIND)
-               str_MOK = int(name_len, MPI_OFFSET_KIND)
-               NVARS_MOK = int(sys_size, MPI_OFFSET_KIND)

And remove the var_MOK = int(i, MPI_OFFSET_KIND) lines inside the sequential-read loops in this branch.

🤖 Prompt for AI Agents

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

In `@src/simulation/m_start_up.fpp` around lines 574 - 581, Remove unused MPI
offset-sized integer calculations and per-loop var_MOK assignments from the
file_per_process branch: WP_MOK, MOK, str_MOK, NVARS_MOK (the int(...)
assignments around WP_MOK..NVARS_MOK) are computed but never used because this
branch uses plain MPI_FILE_READ without MPI_FILE_SET_VIEW; delete those
assignments and also remove any var_MOK = int(i, MPI_OFFSET_KIND) inside the
sequential-read loops so only the values actually consumed by the
file_per_process path remain.

src/simulation/m_data_output.fpp (1)

894-901: WP_MOK (and var_MOK) are dead assignments in the file_per_process write branch.

No disp is computed and no MPI_FILE_SET_VIEW is called in this branch; all writes go through sequential MPI_FILE_WRITE_ALL. WP_MOK, MOK, str_MOK, NVARS_MOK (lines 898–901) and the var_MOK assignments inside every loop are unused.

🤖 Prompt for AI Agents

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

In `@src/simulation/m_data_output.fpp` around lines 894 - 901, The WP_MOK, MOK,
str_MOK, NVARS_MOK assignments (and the per-loop var_MOK assignments) are dead
in the file_per_process write branch because no disp is computed and no
MPI_FILE_SET_VIEW is used — only MPI_FILE_WRITE_ALL is called; either remove
these unused variables/assignments from the file_per_process branch or, if
per-process file views were intended, compute disp and call MPI_FILE_SET_VIEW
before writes. Specifically, either delete the WP_MOK, MOK, str_MOK, NVARS_MOK
lines and the var_MOK assignments inside the loops in the file_per_process
branch, or implement the missing disp computation and invoke MPI_FILE_SET_VIEW
so those MOK variables are actually used.

🤖 Prompt for all review comments with AI agents

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

Outside diff comments:
In `@src/simulation/m_data_output.fpp`:
- Around line 962-970: The computed byte stride WP_MOK is using working
precision (wp) but the file writes use storage precision (stp); change WP_MOK
initialization to use storage_size(0._stp)/8 so disk offsets match the on-disk
element size (i.e., replace WP_MOK = int(storage_size(0._wp)/8, MPI_OFFSET_KIND)
with WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)), and verify the disp
calculation (disp = m_MOK*max(MOK, n_MOK)*max(MOK, p_MOK)*WP_MOK*(var_MOK - 1))
uses that WP_MOK so offsets align with MPI_FILE_WRITE_ALL which uses mpi_io_p
(stp).
- Around line 1085-1105: The displacement calculation in
s_write_parallel_ib_data uses WP_MOK (storage_size of real) and a sys_size-based
var_MOK base offset, which is wrong for an ib.dat file of MPI_INTEGERs; replace
WP_MOK/var_MOK with an IB_MOK that represents the byte size of MPI_INTEGER (set
IB_MOK = int(4, MPI_OFFSET_KIND) normally or the appropriate size under
MFC_MIXED_PRECISION) and compute disp as
m_MOK*max(MOK,n_MOK)*max(MOK,p_MOK)*IB_MOK*int(time_step/t_step_save,
MPI_OFFSET_KIND), removing the sys_size (+1) term and the special-case
time_step==0 handling so the initial snapshot is at offset 0 and later snapshots
are spaced by integer-sized blocks.

In `@src/simulation/m_start_up.fpp`:
- Around line 655-670: The byte-offset WP_MOK is being computed using working
precision (storage_size(0._wp)) which is wrong for mixed-precision I/O; change
the WP_MOK assignments to use storage precision instead (storage_size(0._stp))
so WP_MOK matches the actual element size used by mpi_io_p/mpi_io_type; update
every occurrence that sets WP_MOK (e.g., the WP_MOK assignments in
m_start_up.fpp, m_data_output.fpp and m_data_input.f90) so the offset
calculation that uses WP_MOK, MOK, n_MOK, p_MOK, var_MOK/dsp produces correct
byte displacements.

---

Nitpick comments:
In `@src/simulation/m_data_output.fpp`:
- Around line 894-901: The WP_MOK, MOK, str_MOK, NVARS_MOK assignments (and the
per-loop var_MOK assignments) are dead in the file_per_process write branch
because no disp is computed and no MPI_FILE_SET_VIEW is used — only
MPI_FILE_WRITE_ALL is called; either remove these unused variables/assignments
from the file_per_process branch or, if per-process file views were intended,
compute disp and call MPI_FILE_SET_VIEW before writes. Specifically, either
delete the WP_MOK, MOK, str_MOK, NVARS_MOK lines and the var_MOK assignments
inside the loops in the file_per_process branch, or implement the missing disp
computation and invoke MPI_FILE_SET_VIEW so those MOK variables are actually
used.

In `@src/simulation/m_start_up.fpp`:
- Around line 574-581: Remove unused MPI offset-sized integer calculations and
per-loop var_MOK assignments from the file_per_process branch: WP_MOK, MOK,
str_MOK, NVARS_MOK (the int(...) assignments around WP_MOK..NVARS_MOK) are
computed but never used because this branch uses plain MPI_FILE_READ without
MPI_FILE_SET_VIEW; delete those assignments and also remove any var_MOK = int(i,
MPI_OFFSET_KIND) inside the sequential-read loops so only the values actually
consumed by the file_per_process path remain.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between adf2515 and 6a3aa5d.

📒 Files selected for processing (5)

• src/post_process/m_data_input.f90
• src/pre_process/m_data_output.fpp
• src/pre_process/m_start_up.fpp
• src/simulation/m_data_output.fpp
• src/simulation/m_start_up.fpp

🚧 Files skipped from review as they are similar to previous changes (2)

• src/pre_process/m_data_output.fpp
• src/post_process/m_data_input.f90

[coderabbitai]

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_start_up.fpp (1)

634-639: ⚠️ Potential issue | 🔴 Critical

Use stp instead of wp for MPI I/O byte size calculations.

Line 638 should derive WP_MOK from storage precision (stp), not working precision (wp). In mixed-precision builds where stp ≠ wp, using wp produces incorrect displacements for mpi_io_p-based MPI file operations, corrupting restart file I/O. The same issue exists across all data I/O modules.

🔧 Corrected line

-                WP_MOK = int(storage_size(0._wp)/8, MPI_OFFSET_KIND)
+                WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)

🤖 Prompt for AI Agents

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

In `@src/simulation/m_start_up.fpp` around lines 634 - 639, WP_MOK is computed
using working precision (wp) which yields incorrect byte counts when storage
precision (stp) differs; change the computation of WP_MOK to use storage
precision by calling storage_size(0._stp)/8 (i.e., replace storage_size(0._wp)/8
with storage_size(0._stp)/8) so MPI displacement math uses the actual on-disk
element size; update the same pattern anywhere else in I/O modules where
storage_size was invoked with _wp instead of _stp (look for variables like
WP_MOK and mpi I/O sizing calculations).

🤖 Prompt for all review comments with AI agents

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

Outside diff comments:
In `@src/simulation/m_start_up.fpp`:
- Around line 634-639: WP_MOK is computed using working precision (wp) which
yields incorrect byte counts when storage precision (stp) differs; change the
computation of WP_MOK to use storage precision by calling storage_size(0._stp)/8
(i.e., replace storage_size(0._wp)/8 with storage_size(0._stp)/8) so MPI
displacement math uses the actual on-disk element size; update the same pattern
anywhere else in I/O modules where storage_size was invoked with _wp instead of
_stp (look for variables like WP_MOK and mpi I/O sizing calculations).

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a3aa5d and 1189fa2.

📒 Files selected for processing (1)

• src/simulation/m_start_up.fpp

[coderabbitai]

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_data_output.fpp (1)

894-899: ⚠️ Potential issue | 🟠 Major

Use storage_size(0._stp)/8 for WP_MOK to match MPI_IO_DATA%var(i)%sf precision.
The data written at these lines is real(stp) (storage precision), but WP_MOK is computed from wp (working precision). In mixed-precision mode, this causes incorrect file displacements. Per the codebase convention, mpi_io_p corresponds to stp, so the multiplier must also derive from stp.

🔧 Suggested fix

-            WP_MOK = int(storage_size(0._wp)/8, MPI_OFFSET_KIND)
+            WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)

Also applies to: 962–967

🤖 Prompt for AI Agents

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

In `@src/simulation/m_data_output.fpp` around lines 894 - 899, WP_MOK is computed
using working precision (wp) but the MPI writes use storage precision (stp),
causing wrong displacements; change the calculation of WP_MOK to use
storage_size(0._stp)/8 instead of storage_size(0._wp)/8 so WP_MOK matches
MPI_IO_DATA%var(i)%sf and the real(stp) data layout. Update both occurrences
where WP_MOK is set (the block setting m_MOK, n_MOK, p_MOK, WP_MOK, MOK and the
later similar block around lines 962–967) to derive WP_MOK from stp rather than
wp. Ensure the symbol WP_MOK is the only change so other MOK, m_MOK, n_MOK,
p_MOK calculations remain untouched.

🤖 Prompt for all review comments with AI agents

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

Outside diff comments:
In `@src/simulation/m_data_output.fpp`:
- Around line 894-899: WP_MOK is computed using working precision (wp) but the
MPI writes use storage precision (stp), causing wrong displacements; change the
calculation of WP_MOK to use storage_size(0._stp)/8 instead of
storage_size(0._wp)/8 so WP_MOK matches MPI_IO_DATA%var(i)%sf and the real(stp)
data layout. Update both occurrences where WP_MOK is set (the block setting
m_MOK, n_MOK, p_MOK, WP_MOK, MOK and the later similar block around lines
962–967) to derive WP_MOK from stp rather than wp. Ensure the symbol WP_MOK is
the only change so other MOK, m_MOK, n_MOK, p_MOK calculations remain untouched.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1189fa2 and bbfef04.

📒 Files selected for processing (5)

• src/post_process/m_data_input.f90
• src/pre_process/m_data_output.fpp
• src/pre_process/m_start_up.fpp
• src/simulation/m_data_output.fpp
• src/simulation/m_start_up.fpp

🚧 Files skipped from review as they are similar to previous changes (3)

• src/simulation/m_start_up.fpp
• src/post_process/m_data_input.f90
• src/pre_process/m_data_output.fpp

[coderabbitai]

🧹 Nitpick comments (1)

src/simulation/m_start_up.fpp (1)

578-578: stp-based WP_MOK is correct, but the value goes unused in this branch

Using storage_size(0._stp)/8 is the right choice: all data reads in both branches use mpi_io_p, which corresponds to stp. Good fix.

However, within the file_per_process block, WP_MOK (along with m_MOK, n_MOK, p_MOK, var_MOK) is assigned but never consumed — there are no MPI_FILE_SET_VIEW calls and no disp displacement formula in this path (lines 584–616 are all plain sequential MPI_FILE_READ). The computation is dead code here. If offset-based seeks are not needed for the per-process file layout, these assignments could be removed to reduce confusion; if they are intended for future use, a comment would help.

🤖 Prompt for AI Agents

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

In `@src/simulation/m_start_up.fpp` at line 578, WP_MOK and the related m_MOK,
n_MOK, p_MOK, var_MOK are computed in the file_per_process branch but never used
(no MPI_FILE_SET_VIEW or disp-based reads); either remove these dead assignments
or clearly document why they remain for future use. Locate the assignments to
WP_MOK (storage_size(0._stp)/8, MPI_OFFSET_KIND) and the
m_MOK/n_MOK/p_MOK/var_MOK setup in the file_per_process branch and either (A)
delete them so the branch only contains the sequential MPI_FILE_READ logic using
mpi_io_p, or (B) add a one-line comment above them referencing mpi_io_p and
explaining they are intentionally unused placeholders for future offset-based
views.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@src/simulation/m_start_up.fpp`:
- Line 578: WP_MOK and the related m_MOK, n_MOK, p_MOK, var_MOK are computed in
the file_per_process branch but never used (no MPI_FILE_SET_VIEW or disp-based
reads); either remove these dead assignments or clearly document why they remain
for future use. Locate the assignments to WP_MOK (storage_size(0._stp)/8,
MPI_OFFSET_KIND) and the m_MOK/n_MOK/p_MOK/var_MOK setup in the file_per_process
branch and either (A) delete them so the branch only contains the sequential
MPI_FILE_READ logic using mpi_io_p, or (B) add a one-line comment above them
referencing mpi_io_p and explaining they are intentionally unused placeholders
for future offset-based views.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bbfef04 and b047d7f.

📒 Files selected for processing (3)

• src/post_process/m_data_input.f90
• src/simulation/m_data_output.fpp
• src/simulation/m_start_up.fpp

🚧 Files skipped from review as they are similar to previous changes (2)

• src/simulation/m_data_output.fpp
• src/post_process/m_data_input.f90

[anandrdbz]

This is correct, change should be added

[github-actions]

Claude Code Review

Head SHA: 43bbd2b
Files changed: 6

• src/post_process/m_data_input.f90
• src/pre_process/m_data_output.fpp
• src/pre_process/m_start_up.fpp
• src/simulation/m_data_output.fpp
• src/simulation/m_rhs.fpp
• src/simulation/m_start_up.fpp

Summary

• Replaces all 10 occurrences of hardcoded int(8._wp, MPI_OFFSET_KIND) (and one int(4._wp, ...)) with int(storage_size(0._stp)/8, MPI_OFFSET_KIND), correctly deriving the byte stride from the actual storage precision at compile time.
• The switch from wp to stp in storage_size is correct per MFC conventions: field arrays use stp for storage/I/O, so the file offset stride must reflect stp, not wp.
• Fixes a copy-paste bug in m_rhs.fpp where dq_prim_dy_vf was passed as the z-gradient argument instead of dq_prim_dz_vf.
• Includes a bonus typo fix: equilirium → equilibrium.

---

Findings

1. Pre-existing bug visible in diff context — n_MOK/p_MOK use wrong variable (not introduced by this PR)
src/simulation/m_start_up.fpp ~line 575:

m_MOK = int(m_glb_read + 1, MPI_OFFSET_KIND)
n_MOK = int(m_glb_read + 1, MPI_OFFSET_KIND)   ! should be n_glb_read?
p_MOK = int(m_glb_read + 1, MPI_OFFSET_KIND)   ! should be p_glb_read?

Both n_MOK and p_MOK are assigned using m_glb_read rather than n_glb_read/p_glb_read. If this read path is used for any multi-dimensional case, the MPI-IO layout will be wrong. This was not introduced by this PR, but it is exposed by the diff context. Worth investigating whether it is intentional (e.g., 1D-only read path) or a latent bug.

2. Variable name WP_MOK is now semantically stale — minor, not a blocker
After this fix WP_MOK reflects stp (storage precision) size, but the name still says WP (working precision). Since I/O offset strides should track stp, the new value is more correct, but the name could mislead future readers. Consider a follow-up rename to STP_MOK for clarity.

---

Overall

The WP_MOK fix and the dq_prim_dz_vf fix are both correct and well-motivated. storage_size(...)/8 is standard Fortran 2008 (compatible with all CI-gated compilers) and the integer arithmetic is safe — no overflow risk for the small byte-size values involved. Approve once the n_MOK/p_MOK question in item 1 is confirmed as intentional.