Repository navigation
Fix invalid integration comparisons and reference-generation failures - #8138
cheerly-pku wants to merge 3 commits into
Conversation
|
some conflicts need to be solved. @cheerly-pku |
Resolve validation-script conflicts with LibRPA producer changes: retain vxc.ref/vxc.txt naming and comparison failure propagation; preserve upstream MPI launcher flags, dedicated-validator mode and case-list filtering. Register script regressions when a Python interpreter is available without restoring a hard configure dependency. Verified compare_file 7/7, expanded catch_properties shell regression, CTest 2/2 via the actual tests/integrate CMake subdirectory, upstream LibRPA parser 8/8, Python-disabled configuration, shell syntax and whitespace checks. No numerical references or tolerances changed; no PPCG source commits included. Full ABACUS build/runtime integration not rerun for this script-only resolution.
Thanks for the reminder! The conflicts have been resolved. @mohanchen |
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
Static review of 6575824415cf3edf3a7c56e6d13be2a5b44799e5, with independent static verification. One issue is noted inline. Pinned governance and whitespace checks passed; builds and runtime tests were not run. CI diagnostics were unavailable through the selected account connector.
| if test -z $1 | ||
| then | ||
| $CATCH_SCRIPT result.out | ||
| bash -e "$CATCH_SCRIPT" result.out |
There was a problem hiding this comment.
[P2] Propagate collector failure before checking partial results
Please check the collector exit status in both normal and debug modes before calling check_out. For 101_PW_W90, a missing OUT.autotest/diamond.amn now makes bash -e stop after the energy entries; if those match, check_out visits only that partial result.out, skips AMN/MMN/EIG validation, and can finish successfully. Return nonzero on collection failure, as the reference branch does, and add ordinary/debug regression coverage.
There was a problem hiding this comment.
Thanks for catching this. Fixed in cd55ddc: both normal and debug modes now exit nonzero if the collector fails, before calling check_out. Added regression coverage for missing Wannier90 output after a valid partial result.out is written. The regression catches the old behavior, and the updated shell regression, comparison tests, syntax, whitespace, and governance checks all pass.
What's changed?
Integration comparisons can currently accept extra unmatched data lines or
non-finite values. Reference generation can also record a failed comparison as
an expected result, or report success after a collector failed. This change
rejects those invalid comparisons and makes reference generation fail with a
nonzero status while removing incomplete
result.reffiles.The collectors preserve failed checks as status entries in ordinary
result.outoutput. Numerical tolerances and the existing Wannier90 entry-wisecomparison policy are unchanged. Trailing blank lines are ignored; missing or
shifted data and interior blank lines remain significant.
Linked Issue
No separate issue. Extracted from #8042 following review so changes affecting
all integration tests can be reviewed independently of PPCG. This PR contains
no solver implementation or reference-value changes.
Unit Tests and/or Case Tests for my changes
Commands rerun for merge commit
657582441(mergingdevelopat8b546bdf3, without rebasing or force-pushing):Results: 7/7 comparison tests and 8/8 upstream LibRPA parser tests pass;
the expanded shell regression and syntax check exit 0; the PR-diff governance
check reports no findings; diff whitespace check passes. The shell tests
exercise missing outputs, failed comparisons, missing/malformed descriptors,
cleanup of partial references, and
run_check.sh/Autotest.shfailure status.System Python with NumPy was selected for the malformed-descriptor test.
Conflict resolution preserves upstream
vxc.ref/OUT.autotest/vxc.txtnaming while retaining failed-comparison handling. New shell coverage checks
both Gamma-only and multi-k VXC success/mismatch/missing-output cases, plus
upstream's
-pdedicated-validator mode, custom MPI launcher and pre/postflags, OFML single-rank override, and commented/blank case-list entries.
No reference values, tolerances, or PPCG solver changes are included.
The actual
tests/integrate/CMakeLists.txtwas also exercised through atemporary CMake driver (
project(... NONE),enable_testing(),ABACUS_TEST_DIR=<checkout>/tests, andadd_subdirectoryofintegrate):PATH=/usr/bin:/bin cmake -S /tmp/pr8138-conflict-check.5hibZt -B /tmp/pr8138-conflict-check.5hibZt/with-python -DPython3_EXECUTABLE=/usr/bin/python3 PATH=/usr/bin:/bin OMP_NUM_THREADS=1 ctest --test-dir /tmp/pr8138-conflict-check.5hibZt/with-python -V -R '^(compare_file_test|catch_properties_test)$' PATH=/usr/bin:/bin cmake -S /tmp/pr8138-conflict-check.5hibZt -B /tmp/pr8138-conflict-check.5hibZt/no-python -DCMAKE_DISABLE_FIND_PACKAGE_Python3=TRUE ctest --test-dir /tmp/pr8138-conflict-check.5hibZt/no-python -NBoth registered regression tests pass (CTest 2/2). Python-disabled configuration
also succeeds and explicitly skips those two tests, preserving upstream's
removal of the hard Python configure dependency. The ordinary integration-test
entry remains registered; it was not run by this temporary driver.
The full ABACUS integration suite was not rerun on this standalone branch;
these focused regressions exercise the validation scripts without an ABACUS
build. CI should additionally verify their behavior with the existing cases.
Governance Notes
tests/integrate.Reminder
AGENTS.mdanddocs/developers_guide/agent_governance.md.source/changes.