Fix post_process silo coord corruption from assumed-shape re-mapping#1388
Closed
drcole17 wants to merge 22 commits intoMFlowCode:masterfrom
Closed
Fix post_process silo coord corruption from assumed-shape re-mapping#1388drcole17 wants to merge 22 commits intoMFlowCode:masterfrom
drcole17 wants to merge 22 commits intoMFlowCode:masterfrom
Conversation
Batch runs get a .out log file from the scheduler, but interactive
runs had no equivalent. Wrap the interactive command with tee so
output streams to both the terminal and {name}.out.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- New 2D axisymmetric drainage validation case (Phase 1) - New 3D MUSCL+THINC variant (a-g parameter sweep) - Update existing 3D WENO-Z case - Ignore *.md at repo root
Reads silo output via mfc.viz and reports cells per diameter and interface thickness (0.05<=alpha_1<=0.95) for a chosen step, with a |grad alpha| cross-check, a midplane sanity slice, and an interface-thickness time series. Works on both IC-check (t_step_stop=1) and full runs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add droplet-coalescence analysis notebook
Lead with cell-resolved views before any derived scalars: (1) midplane pcolormesh with mesh lines, (2) close-up on the right-droplet outer face with per-cell alpha values annotated in the 0.05-0.95 interface band, (3) x-ray profile as step-style per-cell values with cell boundaries drawn. Thickness extraction + |grad alpha| cross-check moved after the plots. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Rework analysis notebook: show mesh before extraction
- interface_thickness_x_ray: raise RuntimeError (not bare ValueError) when the x-ray mask covers no cells - x-ray plot, derived-thickness, and time-series cells catch (RuntimeError, ValueError) and guard against empty slices / zero gradients - Summary cell uses locals().get() so it renders even if earlier cells raised Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Harden droplet analysis notebook against empty slices
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>
Contributor
ⓘ 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. |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis pull request introduces new droplet coalescence simulation examples (2D axisymmetric and 3D variants) with corresponding Python case generators and a post-processing analysis notebook. It fixes a serial grid reading bug in the Fortran data input module where array section slicing is now explicitly passed to prevent index remapping issues. The run script is enhanced to compute working directory once and add Unix interactive logging via tee output redirection. A 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 60 minutes.Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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