Repository navigation
Address leftover review comments from merged PRs #2083
Copy link
Copy link
Open
Labels
pr-to-doFix already written in a closed PR; reopen when review capacity allowsFix already written in a closed PR; reopen when review capacity allows
Description
Follow-ups to review comments left open on PRs that are already merged. Each item was checked against current master before it was changed. Results do not change: the Fortran edits are comments plus one explicit real(wp) cast, and index.html is whitespace only (git diff -w is empty).
Bug
- ci: identify faulted node via sacct when the .out marker is missing #1835,
submit-slurm-job.sh(thread): thesacctfallback that finds the faulted node had no|| true. Underset -euo pipefail, a failingsacctended the script without a message on the node-recovery path. It now ends in|| true, like the othersacctcall (~L198).
Minor
- Allow a GRCBC inflow to start up smoothly #1850, inflow ramp (thread): f(t0) = f0 + (1 − f0)(1 + tanh(−3))/2, which is f0 only to within about 0.25%·(1 − f0).
case.mdnow says "≈ f0", and the comment inm_derived_types.fppsays the values at t0 and t0 + τ are approximate. - Require the full inflow state when grcbc_in is enabled #1854,
case_validator.py(thread): thegrcbc_ininflow fields are built as onerequiredlist, andmissingis computed from it in a single pass. The fields and the error message order are unchanged. - Choose ib_neighborhood_radius from the thinnest rank, not the widest #1866,
examples/3D_ibm_neighborhood_radius/case.py(thread):SUMMARYno longer hard-codes 16 x 2 x 2. It readsTOPOLOGY(default16,2,2), computes the rank count and the extent in each direction, and uses the thinnest rank in any direction. With the default topology the output is byte-identical. - Add the thin-plate force case that measures issue 1849 #1868,
examples/2D_ibm_thin_plate_force/case.py(thread): removed the unusedLEVELS. The comment above it referred to "L4", so it now says NCELL=16. - Extract m_eos and add a single EOS family registry #1870,
case_validator.py(thread):value or 0.0is replaced by an explicitis Nonecheck. - Probe the node with a solver binary, not just syscheck #1871,
preflight.sh(thread): the"Illegal instruction"text match is kept on purpose. A comment now explains why: some launchers do not pass exit code 132 on. - Stop amdflang GPU builds relinking the whole device image every time #1904,
CMakeLists.txt(thread 1, thread 2):CMAKE_LINK_DEPENDS_USE_LINKER OFFnow applies only when the compiler isLLVMFlang AND MFC_OpenMP. That combination is the amdflang offload build (MFCTargets.cmake adds--offload-archunder it), so GPU builds behave as before and LLVMFlang CPU builds keep the linker's dependency tracking. - docs: add five flapping-flight and jet runs to the showcase #1912,
docs/index.html(thread 1, thread 2): all 40simsrows are now column-aligned to one width per field. The old rows did not share columns, and the new names are too long to fit the old ones. - test: compile thermochem kernels with the configured Fortran compiler #1943,
test_thermochem.py(thread 1, thread 2): the one-line"Cray"flags dict is now split across lines. The cached compiler probe moved to_probe_compiler(fc), so the cache is keyed on$FCand a changed$FCis probed again. - Label time_data.dat and io_time_data.dat columns by what they measure #1949,
m_start_up.fpp(thread): the comments now saytime_finalis the max over ranks of each rank's fastest RK stage, andio_time_finalis the max over ranks of each rank's means_save_datatime. - Fixed-dt runs: keep the clock on t_step*dt so restarts continue bitwise #1951,
m_start_up.fpp/m_time_steppers.fpp(thread 1, thread 2): removed the "1290 ulps by step 27000" figure and kept the reason. The fixed-dt clock now usesreal(t_step + 1, wp)*dtand(real(t_step, wp) + 0.5_wp)*dt, in boths_perform_time_stepand the IB stage time, so the two stay the same expression. The integer-to-real conversion is exact either way, so the values are bitwise unchanged.
Not changed
- docs: add five flapping-flight and jet runs to the showcase #1912, "512 GCDs" (thread): the PR first had
"240-512 GCDs". A later commit on that PR (f6f6ce1, "Update accelerators for Seagull simulation") deliberately set it to"512 GCDs", and the merged PR description matches. There is no case file in the repo to check it against. Going back to the range would undo that deliberate edit, so the entry stays as it is.
Verification
./mfc.sh format: clean./mfc.sh precheck(formatting, spelling, toolchain and source lint, doc refs, param docs, example validation): all 7 checks pass./mfc.sh lint(ruff + toolchain unit tests): 794 passed, 2 skippedpytest toolchain/mfc/test_thermochem.pywith CCEftn: 16 passed, 1 skippedbash -nonpreflight.shandsubmit-slurm-job.sh: OK./mfc.sh validateon both edited examples: pass. Agrcbc_incase with fields removed still reports them in the same order../mfc.sh build -j 8(Frontier, CPU): OK./mfc.sh test -j 8 -% 10on a Frontier CPU node: 72 passed, 0 failed./mfc.sh test -j 8 -o IBM(includes the fixed-dt prescribed-kinematics and moving-IB goldens): 61 passed, 0 failed./mfc.sh test -o <6 grcbc UUIDs>(1D/2D/3D grcbc x/y/z): 6 passed, 0 failed
Status: a fix/implementation for this is already written, in a pull request that has been closed for queue management.
- Closed PR: Address leftover review comments from merged PRs #1959
- Branch:
ai-review-followupson thesbryngelsonfork, at91422f0389(branch retained, not deleted) - Size: +80/-68 across 12 files, 10 commits
- Where it sat: MERGEABLE; CI 19 checks queued (master was merged in today, so CI had not reported yet)
- Reopen with:
gh pr reopen 1959 -R MFlowCode/MFC
It was merge-clean and up to date with master, and it is small.
Tracked with pr-to-do. Filed with Claude Code.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
pr-to-doFix already written in a closed PR; reopen when review capacity allowsFix already written in a closed PR; reopen when review capacity allows
Activity