Fix post_process silo coord corruption from assumed-shape re-mapping#1389
Fix post_process silo coord corruption from assumed-shape re-mapping#1389drcole17 wants to merge 1 commit intoMFlowCode:masterfrom
Conversation
s_read_grid_data_direction declares dummy arguments dimension(-1:) / dimension(0:). When the actual x_cb / dx / x_cc arrays have lower bounds shifted by offset_x%beg or buff_size (set non-zero for format=1 parallel 3D ranks not at the left domain boundary), Fortran's assumed-shape re-mapping makes the dummy's first element correspond to the actual's first storage slot, not the intended interior index. The read then writes the file data into the wrong range of the actual array and leaves the last offset_x%beg interior cell-boundaries uninitialized, which s_populate_grid_variables_buffers later propagates into the right ghost zone and the silo output. Pass explicit slices x_cb(-1:m), dx(0:m), x_cc(0:m) (and y/z equivalents) so the dummy maps to the intended index range regardless of allocation bounds. Regression from PR MFlowCode#902 (June 2025); pre-refactor code used inline read (1) x_cb(-1:m) which avoided the re-mapping. Verified on a 128-rank 3D droplet-coalescence case: 9/128 ranks previously had NaN / extreme values in coord arrays, now 0/128. Most other ranks were silently shifted by 2 cells and undetectable on uniform grids. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
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. Review rate limit: 0/1 reviews remaining, refill in 56 minutes and 54 seconds.Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1389 +/- ##
=======================================
Coverage 64.76% 64.76%
=======================================
Files 71 71
Lines 18713 18713
Branches 1549 1549
=======================================
Hits 12119 12119
Misses 5638 5638
Partials 956 956 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
s_read_grid_data_direction(post_process serial-IO grid read,m_data_input.f90:50) declares dummy argumentsdimension(-1:)/dimension(0:). When the actualx_cb/dx/x_ccarrays have shifted lower bounds (offset_x%beg > 0orbuff_size > 0— set non-zero forformat=1parallel 3D ranks not at the left domain boundary), Fortran's assumed-shape re-mapping makes the dummy's first element correspond to the actual's first storage slot rather than the intended interior index. The read then writes into the wrong range of the actual array and leaves the lastoffset_x%beginterior cell-boundaries uninitialized;s_populate_grid_variables_bufferslater propagates those uninitialized values into the right ghost zone and the silo output.Fix: pass explicit slices
x_cb(-1:m),dx(0:m),x_cc(0:m)(and y/z equivalents) so the dummy maps to the intended index range regardless of the actual's allocation bounds.Background
Regression from #902 (June 2025); pre-refactor code used inline
read (1) x_cb(-1:m)which avoided the re-mapping by writing directly to the actual's correct indices.format=2(binary) is unaffected becausem_global_parameters.fpp:779zeros all offsets whenformat /= 1, so allocations match dummy bounds.parallel_io=Tis also unaffected because that path uses explicit slice assignment instead of the helper.The bug is mostly silent: shifted coords on a uniform grid look identical to the correct grid, so the field data renders fine. It surfaces visibly only when the uninitialized memory the read leaves behind happens to contain NaN or an extreme value, which then gets written to
{step}.silofor that rank.Verification
128-rank 3D droplet-coalescence case,
parallel_io: F,format: 1:| | Bad ranks (NaN / |coord| > 1 m) | Assembled global grid |
| --- | --- | --- |
| Before | 9 / 128 | 193 × 3 × 159 (y collapsed by dedup) |
| After | 0 / 128 | 180 × 120 × 120 (matches
format=2) |Test plan
./mfc.sh format -j 8./mfc.sh precheck -j 8(ran via pre-commit hook)./mfc.sh build -t post_process -j 8format: 1on 128 ranks; all coord arrays clean.🤖 Generated with Claude Code