-
Notifications
You must be signed in to change notification settings - Fork 133
Fix WP_MOK hardcoded to 8 bytes, use storage_size for portability #1187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e0fbf01
b966fee
50e9df4
b06ed51
f8dee0f
42bade2
9b5fe23
b012f15
43bbd2b
9b1c86b
c92f24d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
|
@@ -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) | ||
|
|
@@ -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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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 -30Repository: 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 -20Repository: 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")
EOFRepository: 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 -80Repository: 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 10Repository: 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 -100Repository: 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 -40Repository: 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 -laRepository: 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 -40Repository: 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 -40Repository: 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 -10Repository: 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")
EOFRepository: MFlowCode/MFC Length of output: 996 Use integer element byte size for MPI displacement, not Line 1106 calculates Use |
||
| MOK = int(1._wp, MPI_OFFSET_KIND) | ||
|
|
||
| write (file_loc, '(A)') 'ib.dat' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 50369
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 2043
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 130
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 84
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 3147
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 1999
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 2802
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 806
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 1307
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 141
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 555
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 5883
🏁 Script executed:
Repository: MFlowCode/MFC
Length of output: 2999
IB displacement calculation uses
stp(real precision) instead of integer byte size.Line 180 derives
WP_MOKfromstorage_size(0._stp), which measures real-type bytes. However, IB data (MPI_IO_IB_DATA%var%sf) is stored asinteger_field, which usesinteger(kind=1)in mixed-precision mode or defaultintegerotherwise (seem_derived_types.fpp). Lines 191 and 194 correctly useMPI_INTEGERfor I/O, but the displacement multiplier is wrong.Impact: In
MFC_MIXED_PRECISIONmode,stpis 2 bytes (half-precision real) while IB integers are 1 byte, causing 2× displacement errors. In normal mode,stpmay 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
📝 Committable suggestion