Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/post_process/m_data_input.f90
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impure subroutine s_setup_mpi_io_params(data_size, m_MOK, n_MOK, p_MOK, WP_MOK,
m_MOK = int(m_glb + 1, MPI_OFFSET_KIND)
n_MOK = int(n_glb + 1, MPI_OFFSET_KIND)
p_MOK = int(p_glb + 1, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/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)
Expand Down Expand Up @@ -177,7 +177,7 @@ impure subroutine s_read_ib_data_files(file_loc_base, t_step)
n_MOK = int(n_glb + 1, MPI_OFFSET_KIND)
p_MOK = int(p_glb + 1, MPI_OFFSET_KIND)
MOK = int(1._wp, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find IB file read/write sites and displacement math
rg -n -C3 "ib\.dat|MPI_IO_IB_DATA|WP_MOK|MPI_FILE_SET_VIEW|MPI_FILE_(READ|WRITE)|MPI_INTEGER|storage_size\(0\._stp\)|storage_size\(0\)" src

Repository: MFlowCode/MFC

Length of output: 50369


🏁 Script executed:

#!/bin/bash
# Get the actual file being reviewed
head -200 src/post_process/m_data_input.f90 | tail -50

Repository: MFlowCode/MFC

Length of output: 2043


🏁 Script executed:

#!/bin/bash
# Find precision type definitions
rg "integer|real|stp|wp|mpi_p|mpi_io_p" src/common/m_global_constants.f90 | head -50

Repository: MFlowCode/MFC

Length of output: 130


🏁 Script executed:

#!/bin/bash
# Find precision type definitions
fd -t f "global_constants\|iso_fortran_env" src/ | head -20

Repository: MFlowCode/MFC

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Search for wp and stp type definitions
rg "^\s*(integer|real)\s*.*::\s*(wp|stp)" --type f90 | head -20

Repository: MFlowCode/MFC

Length of output: 84


🏁 Script executed:

#!/bin/bash
# Search for wp and stp parameter definitions
rg "parameter.*wp|parameter.*stp" src/ | head -30

Repository: MFlowCode/MFC

Length of output: 3147


🏁 Script executed:

#!/bin/bash
# Read the precision_select file to understand wp, stp, and mpi types
cat src/common/m_precision_select.f90

Repository: MFlowCode/MFC

Length of output: 1999


🏁 Script executed:

#!/bin/bash
# Check MPI_IO_IB_DATA structure definition
rg "type.*mpi_io_ib|MPI_IO_IB_DATA" src/ -A 5 | head -40

Repository: MFlowCode/MFC

Length of output: 2802


🏁 Script executed:

#!/bin/bash
# Find the mpi_io_ib_var type definition
rg "type\s*::\s*mpi_io_ib_var|type\s*mpi_io_ib" src/ -B 2 -A 8

Repository: MFlowCode/MFC

Length of output: 806


🏁 Script executed:

#!/bin/bash
# Check integer_field definition and how IB markers are stored
rg "type.*integer_field|type\s*::\s*integer_field" src/ -A 5 | head -20

Repository: MFlowCode/MFC

Length of output: 1307


🏁 Script executed:

#!/bin/bash
# Find the definition of integer_field type
rg "type\s*::\s*integer_field" src/ -B 1 -A 3

Repository: MFlowCode/MFC

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Search for integer_field in derived types
rg "integer_field" src/common/m_derived_types.fpp | head -20

Repository: MFlowCode/MFC

Length of output: 141


🏁 Script executed:

#!/bin/bash
# Get the full integer_field definition with context
rg "type integer_field" src/common/m_derived_types.fpp -B 2 -A 8

Repository: MFlowCode/MFC

Length of output: 555


🏁 Script executed:

#!/bin/bash
# Check the write path for IB data to see if same issue exists there
rg "s_write_parallel_ib_data" src/ -A 30 | head -80

Repository: MFlowCode/MFC

Length of output: 5883


🏁 Script executed:

#!/bin/bash
# Check what storage_size(0) vs storage_size(0._stp) produces in different precision modes
# Find any tests or documentation about integer sizes
rg "storage_size" src/ -B 2 -A 2 | head -40

Repository: MFlowCode/MFC

Length of output: 2999


IB displacement calculation uses stp (real precision) instead of integer byte size.

Line 180 derives WP_MOK from storage_size(0._stp), which measures real-type bytes. However, IB data (MPI_IO_IB_DATA%var%sf) is stored as integer_field, which uses integer(kind=1) in mixed-precision mode or default integer otherwise (see m_derived_types.fpp). Lines 191 and 194 correctly use MPI_INTEGER for I/O, but the displacement multiplier is wrong.

Impact: In MFC_MIXED_PRECISION mode, stp is 2 bytes (half-precision real) while IB integers are 1 byte, causing 2× displacement errors. In normal mode, stp may be 4 or 8 bytes while default integers are 4 bytes, creating potential misalignment.

The same issue exists in the writer (m_data_output.fpp:1106-1121), so reader/writer accidentally agree on the wrong offset, but the file format remains incorrect relative to other fields.

Suggested fix
-                WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)
+                WP_MOK = int(storage_size(0)/8, MPI_OFFSET_KIND)
📝 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
WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0)/8, MPI_OFFSET_KIND)

save_index = t_step/t_step_save ! get the number of saves done to this point

data_size = (m + 1)*(n + 1)*(p + 1)
Expand Down Expand Up @@ -521,7 +521,7 @@ impure subroutine s_read_parallel_conservative_data(t_step, m_MOK, n_MOK, p_MOK,
m_MOK = int(m_glb + 1, MPI_OFFSET_KIND)
n_MOK = int(n_glb + 1, MPI_OFFSET_KIND)
p_MOK = int(p_glb + 1, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/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)
Expand Down
4 changes: 2 additions & 2 deletions src/pre_process/m_data_output.fpp
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ contains
m_MOK = int(m_glb_save, MPI_OFFSET_KIND)
n_MOK = int(n_glb_save, MPI_OFFSET_KIND)
p_MOK = int(p_glb_save, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/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)
Expand Down Expand Up @@ -617,7 +617,7 @@ contains
m_MOK = int(m_glb + 1, MPI_OFFSET_KIND)
n_MOK = int(n_glb + 1, MPI_OFFSET_KIND)
p_MOK = int(p_glb + 1, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/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)
Expand Down
4 changes: 2 additions & 2 deletions src/pre_process/m_start_up.fpp
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ contains
m_MOK = int(m_glb + 1, MPI_OFFSET_KIND)
n_MOK = int(n_glb + 1, MPI_OFFSET_KIND)
p_MOK = int(p_glb + 1, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/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)
Expand Down Expand Up @@ -807,7 +807,7 @@ contains
if (relax) then
if (proc_rank == 0) then
print *, 'initial condition might have been altered due to enforcement of &
& pTg-equilirium (relax = "T" activated)'
& pTg-equilibrium (relax = "T" activated)'
end if

call s_infinite_relaxation_k(q_cons_vf)
Expand Down
6 changes: 3 additions & 3 deletions src/simulation/m_data_output.fpp
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ contains
m_MOK = int(m_glb_save + 1, MPI_OFFSET_KIND)
n_MOK = int(n_glb_save + 1, MPI_OFFSET_KIND)
p_MOK = int(p_glb_save + 1, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/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)
Expand Down Expand Up @@ -976,7 +976,7 @@ contains
m_MOK = int(m_glb + 1, MPI_OFFSET_KIND)
n_MOK = int(n_glb + 1, MPI_OFFSET_KIND)
p_MOK = int(p_glb + 1, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)
MOK = int(1._wp, MPI_OFFSET_KIND)
str_MOK = int(name_len, MPI_OFFSET_KIND)
NVARS_MOK = int(alt_sys, MPI_OFFSET_KIND)
Expand Down Expand Up @@ -1103,7 +1103,7 @@ contains
m_MOK = int(m_glb + 1, MPI_OFFSET_KIND)
n_MOK = int(n_glb + 1, MPI_OFFSET_KIND)
p_MOK = int(p_glb + 1, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify IB marker storage type and byte-size formulas used by IB read/write paths
rg -n -C3 'type\(integer_field\)|ib_markers|MPI_IO_IB_DATA|s_write_parallel_ib_data|s_read_parallel_ib_data|WP_MOK\s*=\s*int\(storage_size' --glob '**/*.{f90,fpp}'

Repository: MFlowCode/MFC

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the s_write_parallel_ib_data subroutine implementation
rg -A 30 'subroutine s_write_parallel_ib_data' src/simulation/m_data_output.fpp | head -50

# Also check the integer_field type definition
rg -B 2 -A 5 'type.*integer_field' src/common/m_global_variables.fpp | head -30

Repository: MFlowCode/MFC

Length of output: 1978


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Get the full s_write_parallel_ib_data including MPI_FILE_WRITE call
rg -A 50 'subroutine s_write_parallel_ib_data' src/simulation/m_data_output.fpp | grep -A 20 'disp ='

# Search for integer_field type definition
rg -B 2 -A 5 'type\s*::\s*integer_field|type.*integer_field' src/ --glob '**/*.fpp' | head -40

# Also check how ib_markers is declared
rg -B 2 -A 2 'type\(integer_field\).*ib_markers' src/ --glob '**/*.fpp' | head -20

Repository: MFlowCode/MFC

Length of output: 4933


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find integer_field type definition - likely in common module
fd -e fpp -e f90 | xargs rg -l 'type.*::.*integer_field' | head -5

# Search for the actual type definition
rg -B 3 'type\s*::\s*integer_field' src/ --glob '**/*.fpp' -A 5

# Check byte sizes: what is storage_size for typical integer vs stp
python3 << 'EOF'
# In Fortran, typical byte sizes:
# integer (default, usually 4 bytes)
# real(8) = double precision = 8 bytes
# real(4) = single precision = 4 bytes
# real(2) = half precision = 2 bytes

# If stp can be real(2) or real(4) while integer is always 4
# Then mismatch in byte calculations could occur

# storage_size returns bits, /8 converts to bytes
print("Typical byte sizes:")
print("  integer (default): 4 bytes → storage_size = 32 bits → /8 = 4 bytes")
print("  real(8) (double): 8 bytes → storage_size = 64 bits → /8 = 8 bytes")
print("  real(4) (single): 4 bytes → storage_size = 32 bits → /8 = 4 bytes")
print("  real(2) (half): 2 bytes → storage_size = 16 bits → /8 = 2 bytes")
print("\nIf stp=real(2) and integer=4 bytes, byte size mismatch occurs")
EOF

Repository: MFlowCode/MFC

Length of output: 41


🏁 Script executed:

#!/bin/bash
set -euoripefail

# Look for integer_field definition more broadly
rg 'type\s*,\s*public\s*::\s*integer_field|type\s*::\s*integer_field' src/ --glob '**/*.fpp' -B 2 -A 10

# Check if there's a read_parallel_ib_data function
rg -B 5 -A 40 's_read_parallel_ib_data' src/simulation/m_data_output.fpp | head -80

Repository: MFlowCode/MFC

Length of output: 731


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for integer_field type definition more broadly
rg 'type\s*,\s*public\s*::\s*integer_field|type\s*::\s*integer_field' src/ --glob '**/*.fpp' -B 2 -A 10

Repository: MFlowCode/MFC

Length of output: 39


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if there's a read_parallel_ib_data function
rg 's_read_parallel_ib_data' src/ --glob '**/*.fpp' -B 5 -A 40 | head -100

Repository: MFlowCode/MFC

Length of output: 39


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for integer_field in all source files
find src -name '*.fpp' -o -name '*.f90' | xargs grep -l 'integer_field' | head -5

# Look at one of those files to find the definition
grep -B 5 -A 10 'type.*integer_field' src/common/m_derived_types.fpp 2>/dev/null | head -40

Repository: MFlowCode/MFC

Length of output: 1359


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for any read operations related to ib data
grep -n 'read.*ib\|s_read.*ib' src/simulation/m_data_output.fpp | head -20

# Look for MPI_FILE_READ in the file
grep -n 'MPI_FILE_READ' src/simulation/m_data_output.fpp | head -20

# Check the whole structure - what files define these types
find src -name '*.fpp' | xargs grep -l 'type.*integer_field' | xargs ls -la

Repository: MFlowCode/MFC

Length of output: 39


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check what stp precision is typically set to
rg 'stp.*=' src/common/ --glob '**/*.fpp' | grep -i 'kind\|precision' | head -10

# Look at the precision module or parameters
rg 'integer.*stp|real.*stp' src/common/m_derived_types.fpp -B 2 -A 2 | head -40

Repository: MFlowCode/MFC

Length of output: 39


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if there's documentation about precision modes
find docs -name '*.md' | xargs grep -l 'stp\|precision\|mixed' 2>/dev/null | head -3

# Look at the learnings we have about precision
echo "=== Checking learnings about precision ==="
echo "stp is storage precision (may be half-precision)"
echo "wp is working precision (usually double)"
echo "IB markers are integers, not floating point"
echo ""

# Verify how var_MOK is computed and used
rg -B 5 -A 15 'var_MOK\s*=' src/simulation/m_data_output.fpp | grep -A 10 'write_parallel_ib'

Repository: MFlowCode/MFC

Length of output: 1223


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Now let's verify the complete logic of the write and understand the displacement calculation
rg -B 10 -A 25 'subroutine s_write_parallel_ib_data' src/simulation/m_data_output.fpp | tail -40

Repository: MFlowCode/MFC

Length of output: 1520


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check what precision modes are available
grep -r 'MFC_MIXED_PRECISION\|kind=1.*integer' src/common/ --include='*.fpp' | head -10

Repository: MFlowCode/MFC

Length of output: 234


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if there's a corresponding read operation for ib_data
rg -n 'read.*ib.*data|s_read_parallel_ib|MPI_FILE_READ.*ib' src/simulation/ --glob '**/*.fpp' | head -20

# Also check m_start_up to see if it reads ib data
rg -B 5 -A 20 'ib_markers\|MPI_IO_IB_DATA' src/simulation/m_start_up.fpp | grep -A 10 'MPI_FILE'

Repository: MFlowCode/MFC

Length of output: 39


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check the learnings summary - what was established about MPI and precision
echo "Checking learnings context..."
echo "Key learning: mpi_p should match wp, mpi_io_p should match stp"
echo "Key learning: stp is storage precision (may be half), wp is working precision"
echo ""

# Verify that integer type size is what matters for MPI_INTEGER
python3 << 'EOF'
print("MPI I/O displacement calculation analysis:")
print("")
print("Current code at line 1106:")
print("  WP_MOK = int(storage_size(0._stp)/8, MPI_OFFSET_KIND)")
print("  disp = m_MOK*max(MOK, n_MOK)*max(MOK, p_MOK)*WP_MOK*(var_MOK - 1 + ...)")
print("  MPI_FILE_WRITE_ALL(..., MPI_INTEGER, ...)")
print("")
print("Issue: WP_MOK uses STP byte size (floating point)")
print("But MPI_FILE_WRITE_ALL uses MPI_INTEGER datatype")
print("")
print("Scenarios where mismatch occurs:")
print("  stp=real(8), integer=4: WP_MOK=8 but integer=4 → offset 2x too large")
print("  stp=real(2), integer=4: WP_MOK=2 but integer=4 → offset 1/2 too small")
print("  stp=real(2), integer=1 (MFC_MIXED_PRECISION): WP_MOK=2, integer=1 → offset 2x too large")
print("")
print("Correct approach: Use storage_size(ib_markers%sf(0,0,0))/8")
print("  Returns actual integer element byte size")
print("  Matches MPI_INTEGER datatype in MPI_FILE_WRITE_ALL")
EOF

Repository: MFlowCode/MFC

Length of output: 996


Use integer element byte size for MPI displacement, not stp size.

Line 1106 calculates WP_MOK from storage_size(0._stp)/8, but this value is used as byte-size multiplier for file offset calculations when writing MPI_INTEGER data via MPI_FILE_WRITE_ALL. Since stp byte size (storage precision for floats) may differ from integer element byte size—particularly in MFC_MIXED_PRECISION mode where integers can be 1 byte—displacement becomes incorrect, corrupting ib.dat restart data across timesteps.

Use storage_size(ib_markers%sf(0, 0, 0))/8 to compute the actual integer element byte size matching the MPI_INTEGER datatype.

MOK = int(1._wp, MPI_OFFSET_KIND)

write (file_loc, '(A)') 'ib.dat'
Expand Down
2 changes: 1 addition & 1 deletion src/simulation/m_rhs.fpp
Original file line number Diff line number Diff line change
Expand Up @@ -1652,7 +1652,7 @@ contains
call s_compute_viscous_stress_cylindrical_boundary(q_prim_vf, &
dq_prim_dx_vf(mom_idx%beg:mom_idx%end), &
dq_prim_dy_vf(mom_idx%beg:mom_idx%end), &
dq_prim_dy_vf(mom_idx%beg:mom_idx%end), &
dq_prim_dz_vf(mom_idx%beg:mom_idx%end), &
tau_Re_vf, &
idwbuff(1), idwbuff(2), idwbuff(3))
end if
Expand Down
4 changes: 2 additions & 2 deletions src/simulation/m_start_up.fpp
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ contains
m_MOK = int(m_glb_read + 1, MPI_OFFSET_KIND)
n_MOK = int(m_glb_read + 1, MPI_OFFSET_KIND)
p_MOK = int(m_glb_read + 1, MPI_OFFSET_KIND)
WP_MOK = int(4._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/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)
Expand Down Expand Up @@ -652,7 +652,7 @@ contains
m_MOK = int(m_glb + 1, MPI_OFFSET_KIND)
n_MOK = int(n_glb + 1, MPI_OFFSET_KIND)
p_MOK = int(p_glb + 1, MPI_OFFSET_KIND)
WP_MOK = int(8._wp, MPI_OFFSET_KIND)
WP_MOK = int(storage_size(0._stp)/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)
Expand Down
Loading