Skip to content

SCC 26: bug: fix parallel I/O failures and performance overhead - #2085

Draft
Tix3Dev wants to merge 2 commits into
MFlowCode:masterfrom
Team-RACKlette:fix-mpi-io-field-sections
Draft

Tix3Dev wants to merge 2 commits into
MFlowCode:masterfrom
Team-RACKlette:fix-mpi-io-field-sections

Conversation

@Tix3Dev

@Tix3Dev Tix3Dev commented Oct 15, 2026 •

Copy link
Copy Markdown

Description

When using parallel I/O in MFC (e.g., when parallel I/O is enabled in case.py), reading and writing operations via MPI-IO can fail, suffer from severe performance overhead, or exhibit incorrect behavior. Specifically, passing whole array pointers (%sf) directly into MPI_FILE_READ/MPI_WRITE calls can lead to unoptimized data layouts, triggering inefficient memory handling or incorrect byte streaming.

Changes

This patch updates the MPI-IO read and write calls across post_process, pre_process, and simulation modules to pass full array sections (%sf(:,:,:)) instead of the base pointers (%sf).

  • Fortran array section semantics ((:,:,:)) force the compiler to ensure the array data is properly continuous or packed according to the expected bounds before handing it over to the MPI library.
  • This resolves failures with parallel I/O and standardizes how field data arrays are referenced during MPI read/write and file view configuration operations.

Performance & Storage Impact

Tested with the 3D_IGR_jet and 3D_TaylorGreenVortex (N = 128)  cases on 4 NVIDIA A100 GPUs across 4 MPI ranks.

Case Runtime (Before) Runtime (After) Disk Space (Before) Disk Space (After)
3D_IGR_jet  3,100 s
Step Size: 50
1,200 s
Step Size: 10
~450 GB
D/ folder
~150 GB
restart_data/
3D_TaylorGreenVortex
Step size: 300
550 s 390 s 83 GB
D/ folder
3 GB
restart_data/

Since the fix doesn't change any numerical code, accuracy and convergence remain unchanged.

Acknowledgments

This fix was developed by Team RACKlette (ETH Zürich) as part of the Student Cluster Competition (SCC) at SC26.

@Tix3Dev
Tix3Dev requested a review from sbryngelson as a code owner October 15, 2026 07:55
@github-actions

Copy link
Copy Markdown

Claude Code Review

Head SHA: 14fbd9a

Files changed:

  • 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

Findings:

  • medium src/simulation/m_data_output.fpp: The hunk at diff line 176 (MPI_FILE_WRITE_ALL(... MPI_IO_DATA%var(sys_size + 1)%sf(:,:,:), ...)) is about 134 columns after the 7-character (:,:,:) was added (127 before). That is over the 132-column free-form Fortran limit. Several other edited calls are close to it. I found no -ffree-line-length-none or -extend-source flag in the top-level CMakeLists.txt. If the compile flags don't relax the limit, gfortran will reject the line. I'm moderately sure of this: I counted the length by hand and did not build. Fix: wrap the call with & continuation, or confirm the compile flags allow longer lines.
  • medium src/pre_process/m_data_output.fpp, src/simulation/m_start_up.fpp, src/post_process/m_data_input.f90: The whole change replaces %sf with %sf(:,:,:) in blocking MPI-IO calls, with no behavior change intended. If sf is a contiguous pointer or allocatable, the section is passed by address and nothing changes. If sf is a pointer without the contiguous attribute, a compiler may make a temporary copy. For the reads, copy-in/copy-out would still be correct because the calls are blocking. That is a performance and memory cost, not a correctness bug. Nothing in the diff says why the change is needed, so please state the motivation (a compiler diagnostic or a workaround) in the PR description. Low confidence that this causes any problem.

./mfc.sh format, as the lint gate requires: one MPI_FILE_WRITE_ALL line exceeded the line limit with the
section, and the original patch had dropped the spaces in an unrelated GPU_UPDATE line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/simulation/m_data_output.fpp 1579 +1
Directory Lines Diff
simulation 28527 +1
total 47586 +1

@codecov

codecov Bot commented Oct 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 41.66667% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.90%. Comparing base (47568a0) to head (9e6ef09).

Files with missing lines Patch % Lines
src/pre_process/m_data_output.fpp 33.33% 4 Missing ⚠️
src/simulation/m_data_output.fpp 57.14% 3 Missing ⚠️
src/simulation/m_start_up.fpp 50.00% 3 Missing ⚠️
src/post_process/m_data_input.f90 33.33% 2 Missing ⚠️
src/pre_process/m_start_up.fpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2085   +/-   ##
=======================================
  Coverage   61.90%   61.90%           
=======================================
  Files          86       86           
  Lines       22885    22885           
  Branches     3369     3369           
=======================================
  Hits        14166    14166           
  Misses       6220     6220           
  Partials     2499     2499           

☔ View full report in Codecov by Harness.
📢 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.

@wilfonba

Copy link
Copy Markdown
Collaborator

@Tix3Dev, your results table isn't very clear. The 3d_IGR_jet results appear to compare serial I/O D/ text file output to binary MPI I/O files, in which case I'm not surprised that the text file-based approach took so much longer and used more space. It's unclear if the 3D_TaylorGreeneVortex results suffer from this same apples-to-oranges comparison. While this is a minor change that doesn't seem to affect much, I would appreciate if you could clarify how your results support your claims before approving this PR.

@wilfonba
wilfonba marked this pull request as draft October 15, 2026 13:49
@Tix3Dev

Tix3Dev commented Oct 15, 2026

Copy link
Copy Markdown
Author

@wilfonba, when running case.py files that contain "parallel_io": "T", we encountered errors (see below) in our particular environment (see below). We therefore had to resort to using "parallel_io": "F", which got rid of the errors but inadvertently resulted in much worse performance. We listed this in the before column, and to show why the fix is crucial, showed the results of using the fix (which got rid of the errors) in combination with "parallel_io": "T" in the after column. So indeed it is an apples-to-oranges comparison, but for the sake of showing what the "best achievable" result is. To better demonstrate the exact failing mechanism, see this minimal working example.

Error (3D_TaylorGreenVortex)

 [  6%]  Time step      156 of 2833 @ t_step =    13684 Time Avg = 1.365524E-01 Time/step= 1.362991E-01 ETA (HH:MM:SS) = 0:06:05
 [  6%]  Time step      157 of 2833 @ t_step =    13685 Time Avg = 1.365485E-01 Time/step= 1.359460E-01 ETA (HH:MM:SS) = 0:06:05
 [  6%]  Time step      158 of 2833 @ t_step =    13686 Time Avg = 1.365436E-01 Time/step= 1.357880E-01 ETA (HH:MM:SS) = 0:06:05
 NaN(s) in timestep output.                        0                        0 
                        0                        1            3        13692 
          127          127          256
 NaN(s) in timestep output.
 NaN(s) in timestep output.                        0                        0 
                        0                        1            1        13692 
          128          127          256
 NaN(s) in timestep output.
 NaN(s) in timestep output.                        0                        0 
                        0                        1            2        13692 
          127          128          256
 NaN(s) in timestep output.
 [  6%]  Time step      159 of 2833 @ t_step =    13687 Time Avg = 1.365416E-01 Time/step= 1.362300E-01 ETA (HH:MM:SS) = 0:06:05
 [  6%]  Time step      160 of 2833 @ t_step =    13688 Time Avg = 1.365385E-01 Time/step= 1.360610E-01 ETA (HH:MM:SS) = 0:06:04
 [  6%]  Time step      161 of 2833 @ t_step =    13689 Time Avg = 1.365341E-01 Time/step= 1.358461E-01 ETA (HH:MM:SS) = 0:06:04
 [  6%]  Time step      162 of 2833 @ t_step =    13690 Time Avg = 1.365302E-01 Time/step= 1.359160E-01 ETA (HH:MM:SS) = 0:06:04
 [  6%]  Time step      163 of 2833 @ t_step =    13691 Time Avg = 1.365293E-01 Time/step= 1.363800E-01 ETA (HH:MM:SS) = 0:06:04
 NaN(s) in timestep output.                        0                        0 
                        0                        1            0        13692 
          128          128          256
 NaN(s) in timestep output.
srun: Job step aborted: Waiting up to 32 seconds for job step to finish.

Environment to reproduce

  • Compilers: NVIDIA HPC SDK 25.7 (nvfortran / nvc / nvc++ 25.7-0), CUDA 12.9
  • MPI: Open MPI 5.0.10, built from source with those compilers (CC=nvc CXX=nvc++ FC=nvfortran),
    with CUDA 12.9, UCX, PMIx and Slurm support (not the MPI bundled with the HPC SDK)
  • MFC: commit e2f0e26, unmodified (the same code is on current master)
  • Run: ./mfc.sh run examples/3D_TaylorGreenVortex/case.py -n 4 --gpu acc on 4 GPUs (H200),
    with parallel_io = 'T' as the example ships. MFC builds with MPI and OpenACC; the run
    hits NaNs at the first save.
  • Without MFC or a GPU: the attached minimal working example, built with mpifort -O2 from the Open MPI above
    and run on one process, shows the misplaced data directly.

Minimal working example output (+env info)

== toolchain
nvfortran 25.7-0 64-bit target on x86-64 Linux -tp znver5 
                Open MPI: 5.0.10
            Fort use mpi: yes (full: ignore TKR)
         Fort ignore TKR: yes (!DIR$ IGNORE_TKR)
== build (optimised, as MFC is built)
== run (one process)
compiler: nvfortran 25.7-0
MPI:      Open MPI v5.0.10, package: Open MPI yvollmeier@sbrinz Distri
interior 192 cells, each holding its own index; halo cells hold -1e-6
 
A  pointer to the interior (MFC's way)   : 0 of 192 values correct, 192 are halo filler   -> SHEARED   
B  contiguous copy of the interior       : 192 of 192 values correct, 0 are halo filler   -> correct   
C  the array section as the argument     : 192 of 192 values correct, 0 are halo filler   -> correct   
D  reading back through the pointer       : 0 of 192 interior cells correct, 192 halo cells overwritten   -> SHEARED   
E  section of the pointer, sf(:,:,:)     : 192 of 192 values correct, 0 are halo filler   -> correct   
F  reading back through sf(:,:,:)         : 192 of 192 interior cells correct, 0 halo cells overwritten   -> correct   

@sbryngelson
sbryngelson removed their request for review October 15, 2026 15:59
@wilfonba

Copy link
Copy Markdown
Collaborator

May I ask why you built OpenMPI from scratch with the NVHPC compilers rather than using the bundled MPI? This seems more like an issue with your custom OpenMPI build than with how MFC writes data.

This branch has not been deployed

No deployments
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.

2 participants