Fix I6 filename/index overflow in multiscale Ac, field-snapshot, restart-dir and k-expand naming - #1270
Open
MZKC wants to merge 2 commits into
Open
Fix I6 filename/index overflow in multiscale Ac, field-snapshot, restart-dir and k-expand naming#1270MZKC wants to merge 2 commits into
MZKC wants to merge 2 commits into
Conversation
…r naming - ms Ac field-snapshot filename used i6.6 for the RT step (itt), overflowing to "******" and colliding/overwriting frames for step >= 1,000,000 (nt>=1e6 runs, e.g. long-pulse Maxwell-TDDFT). Widen to i0.6 (keeps 6-digit zero-padding but grows automatically past 1e6). - generate_restart_directory_name wrote '(A,I6.6,A)' with only one output item (basedir): a format/output-list mismatch (no integer for I6.6) that aborts under the Fujitsu runtime on the final RT-restart write, leaving data_for_restart_rt/ empty. Intended result is gdir = basedir; use '(A)' to match. - generate_checkpoint_directory_name: same i6.6 -> i0.6 widening for iter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Can one of the admins verify this patch? |
…pand restart Extends the multiscale-Ac / restart-dir fixes to the remaining instances of the same two bug classes found by an audit of src/io: - write_field.f90: the RT step counter `itt` was formatted with `i6.6` when building real-space snapshot filenames (_dns_, _dnsdiff_, microscopic current/magnetization/spin-current, _elf_, _Exsta_, and per-step ion/field .bin). For itt >= 1,000,000 these overflow to `******`, colliding every late-time frame onto one filename. Widened to `i0.6` (identical output below 1e6, grows automatically past it). The MPI-rank `id_r` writes are left at i6.6 (bounded by nproc, not a step counter). - main_dft_k_expand.f90 / main_dft_k_expand_slice.f90: three copies of the `write(gdir,'(A,I6.6,A)') trim(...)` format/output-list mismatch (three edit descriptors, one output item, no integer argument for I6.6) -- the same defect fixed in generate_restart_directory_name, which aborts under the Fujitsu runtime when the k-expand restart path runs. The intended result is simply gdir = trim(basedir/odir/directory_read_data); fixed with '(A)'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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
Fixed-width integer edit descriptors used to build filenames and restart/checkpoint directory names in
src/ioandsrc/ms. Two classes:i6.6filename/index overflow — a 6-digit zero-padded step/iteration counter overflows to******once the counter reaches1,000,000, silently colliding every subsequent frame onto one filename. Real functional bug for long runs (nt >= 1e6).write(x,'(A,I6.6,A)') trim(...): a format with anI6.6(and trailingA) edit descriptor but only one character output item, i.e. no integer argument for theI6.6. This is nonconforming Fortran; today it happens to produce the intendedx = trim(...)via format reversion (format control terminates at the first data-edit descriptor with no matching item), so it is not currently observed to fail, but it is fragile and should be corrected.1. Multiscale
Acfield-snapshot filename overflow (src/ms/main_ms.f90) — class 1For
itt >= 1,000,000the field overflows toAc_******.data, so every frame for steps1e6 ... ntcollides on the same filename and overwrites the previous one — the late-time field snapshots of long-pulse multiscale runs are silently lost. Fixed by widening toi0.6(identical 6-digit zero-padding below1e6, grows automatically past it). Logic/compile-verified (nont >= 1e6live run in this PR).2. Real-space snapshot filenames in
write_field.f90— class 1The RT step counter
ittis formatted withi6.6in nine snapshot filename builders (_dns_,_dnsdiff_, microscopic current / magnetization / spin-current,_elf_,_Exsta_, and the per-step ion / field.bin). Sameitt >= 1e6overflow as (1). Widened toi0.6. The MPI-rankid_rwrites are intentionally left ati6.6(bounded bynproc, not a growing step counter).3. Checkpoint-directory iteration index (
src/io/checkpoint_restart.f90) — class 1generate_checkpoint_directory_nameformatted the checkpointiterwithi6.6; widened toi0.6(same overflow class; would bitent >= 1e6runs with periodic checkpointing).4. Format / output-list mismatches — class 2
generate_restart_directory_name(src/io/checkpoint_restart.f90) and three copies in the k-expand restart helpers (src/io/main_dft_k_expand.f90:346,src/io/main_dft_k_expand_slice.f90:320,340) all built a directory string withwrite(gdir,'(A,I6.6,A)') trim(...)— three edit descriptors, one output item, no integer for theI6.6. Replaced with the conformingwrite(gdir,'(A)') trim(...)(identical result,gdir = trim(basedir/odir/directory_read_data)).Scope note (what this PR does NOT fix)
An earlier draft of this PR attributed the production RT restart-write abort (
data_for_restart_rt/left empty,MPI_File_open/CODE=1907at the RT end-of-run checkpoint) to thegenerate_restart_directory_namemismatch in (4). A real-hardware GS→RT validation on the fixed binary showed that is not the cause: GS restart succeeds and the RT restart write still aborts identically with the (4) fix in place. That abort is a separate, independent bug — an uninitializedofl%dir_out_restartpassed to the RT checkpoint becauseinitialization_rtcallsinit_dftwith a throwaway local instead of the returnedofl— and is addressed in a separate PR. The changes here are I6-overflow fixes (real fornt >= 1e6) and format-conformance cleanups; they are not tied to that RT-restart abort.Verification
i0.6behavior:12345 -> 012345,999999 -> 999999,1000000 -> 1000000,1234567 -> 1234567(no overflow, sortable).(4)result unchanged:gdir = trim(basedir); callers consumegdirdirectly (wdir = gdir, thenpdir = trim(gdir)//'rank_'...), no caller expects an appended index; no reader parses these filenames for the step index, so the widenedi0.6does not break globbing.mpifrtpx,--arch=fujitsu-a64fx-ea).data_for_restart/fully populated).