Repository navigation
Fix #8091: Test: split catch_properties.sh into modular validation collectors - #8090
Merged
Merged
Conversation
added 9 commits
October 7, 2026 07:57
…flow
Let integrate tests read the ABACUS executable from a single ABACUS_EXE
environment variable instead of editing two files:
- Autotest.sh defaults 'abacus' to ${ABACUS_EXE:-abacus}; '-a' still overrides.
- general_info uses EXEC ${ABACUS_EXE:-abacus}; run_check.sh expands $VAR/${VAR}
in the EXEC value from the environment.
Both fall back to 'abacus' from PATH when ABACUS_EXE is unset, matching the
existing CI behavior (ctest '-a' and container-installed abacus are unaffected).
Also document the test workflow in tests/README: how to point at an executable
(priority: -a flag > ABACUS_EXE > hard-coded value > PATH), and the role of the
per-category CASES_CPU.txt / CASES_GPU.txt lists. Remove the two empty
placeholder CASES_*.txt under integrate/, which are unused.
Verified: run_check.sh EXEC expansion resolves to 'abacus' when ABACUS_EXE is
unset and to the given path when set.
The compare-wfc subcommand and its exclusive helpers (read_wfc_component,
phase_aligned_error, parse_wfc_groups) are not called anywhere in the repo;
fingerprint-wfc uses the separate read_wfc_components. Remove the dead code
(~90 lines). integrate / fingerprint-wfc / check-spinor are unaffected.
Verified: python3 -m ast parse OK; --help lists only
{integrate, fingerprint-wfc, check-spinor}; 'compare-wfc' now errors as an
invalid choice.
Begin splitting the ~1000-line catch_properties.sh into per-category modules. This first step extracts the shared infrastructure into props_common.sh and turns the entry point into a thin wrapper: - PROPS_TOOLS_DIR resolves the tools dir via BASH_SOURCE so modules no longer depend on the caller's CWD - shared constants COMPARE_SCRIPT / CUBE_TOOL / COLLECT_NPY_MEANS - shared helpers sum_file / get_input_key_value / sanitize_result_key / record_compare_result - props_init(): one-time INPUT switch parsing (has_*/out_*/nspin/...) plus result file truncation; the dead vars file/has_dftu/has_r/base are not carried over - props_finalize(): writes the trailing totaltimeref line catch_properties.sh sources the library, calls props_init "$1", and keeps all property blocks inline for now; they move out in steps 2-6. Verified: bash -n passes; regenerated result output identical to pre-split baseline (modulo totaltimeref) for 01_PW/057_PW_SO_IW, 01_PW/035_PW_15_SO, 09_DeePKS/09_NO_GO_deepks_basic.
Move the basic property blocks (total energy, collinear magnetism, force, stress, DOS, Onsager) out of catch_properties.sh into run_basic_props() in props_basic.sh. Three basic blocks originally sat between blocks destined for other modules, so they become position-preserving hooks: - run_basic_props_post_ml(): imp_sol energies - run_basic_props_post_deepks(): point/space/magnetic group + nkibz - run_basic_props_post_rdmft(): running_<calculation>_*.log names The inline trailing totaltimeref block is replaced by the existing props_finalize() helper. Block bodies are moved verbatim and keep appending to the result file in the original order. Verified: bash -n passes; output identical to step-1 baseline on 01_PW/057_PW_SO_IW, 01_PW/035_PW_15_SO, 09_DeePKS/09_NO_GO_deepks_basic, plus a synthetic case exercising imp_sol/symmetry/alllog branches.
Move the matrix/operator blocks out of catch_properties.sh into
props_mat.sh:
- run_mat_dm1_props(): out_dm1 DMR comparison
- run_mat_props(): S(k)/H(k), H(R)/S(R), VXC, separated eband
terms, NPZ existence, r/T/SYNS/dH(R) and
dH(k) term matrix comparisons
- run_mat_dm_props(): out_dm density matrix comparison
The three hooks preserve the original interleaving with the cube
collectors that still live inline (moved in step 4). The hard-coded
"../../integrate/tools" paths to compare_hsk_binary.py and
compare_hsr_binary.py are replaced by $PROPS_TOOLS_DIR while moving.
Verified: bash -n passes; output identical to the step-2 baseline on
the three reference cases plus synthetic cases covering the basic
post-hooks and an out_band matrix comparison.
Move the real-space cube and wave-function blocks out of
catch_properties.sh into props_cube.sh:
- run_cube_pot_props(): out_pot=1/2 potential cubes and out_elf
- run_cube_props(): chg cube, SCAN tau, LDOS, wfc real-space
variance, PW wfc maxima, LCAO wfc comparison
- run_cube_tail_props(): mulliken, pchg cube loop, get_wf/get_pchg and
PW norm/Re-Im cube integration/fingerprints,
nspin=4 pointwise spinor identity check
The three hooks wrap the out_dm matrix block exactly as in the original
script, preserving result-line order. Block bodies move verbatim; only
the result target changes from $1 to $props_result_file.
Verified: bash -n passes; output identical to the step-3 baseline on
the three reference cases and synthetic basic/matrix/cube cases.
Move the advanced-method blocks out of catch_properties.sh into props_ml.sh: - run_ml_descriptor_props(): MLKEDF .npy descriptor means - run_ml_rpa_props(): Etot_without_rpa plus librpa ref compares - run_ml_lr_props(): linear-response excitation energies - run_ml_rdmft_props(): RDMFT energy-term extraction Entry point now only orchestrates hooks in the original emission order. Block bodies move verbatim; only the result target changes from $1 to $props_result_file. Verified: bash -n passes; output identical to the step-4 baseline on the three reference cases and synthetic basic/matrix/cube/LR/RDMFT cases.
- props_tddft.sh: run_tddft_props() for the rt-TDDFT current, efield and vecpot comparisons. - props_deepks.sh: process_npy/process_many_npys plus run_deepks_props(), reusing the props_common.sh helpers instead of duplicating sum_file/get_input_key_value/COMPARE_SCRIPT. The "../tools/get_sum_*.py" calls stay CWD-relative on purpose: from a tests/09_DeePKS/<case> CWD they resolve to tests/09_DeePKS/tools/. - catch_deepks_properties.sh becomes a thin standalone entry that sources the common library + props_deepks.sh; it deliberately skips props_init() so manual invocation appends instead of truncating. - catch_properties.sh calls run_deepks_props/run_tddft_props in process; the DeePKS subprocess boundary disappears. Verified: bash -n passes; entry-point output identical to the step-5 baseline on the three reference cases and all five synthetic cases; the new standalone DeePKS entry reproduces the old one output exactly.
- Drop the redundant get_s alias of $calculation from props_init(); the S(R) overlap check in props_mat.sh now tests $calculation directly (identical trigger: get_s was a second read of the same INPUT key). - Remove the dead 'test -z "$dmfile"' branch in run_mat_dm_props: dmfile is assigned a non-empty literal, so the branch could never fire; a missing dm file is reported through the CompareFile.py exit status like every other matrix block. Verification: bash -n on all nine shell files; the collector output of all 384 test-case directories on disk that contain OUT.autotest results was diffed against the step-6 baseline (exit codes and stdout, modulo totaltimeref) -- 384/384 identical, plus synthetic cases for get_s/scf triggering, basic, cube, LR, RDMFT and DeePKS.
mohanchen
added
Refactor
…tion The directory contains only validation/comparison scripts (collectors, CompareFile.py, cube_tool.py, run_check.sh), never result data, so give it a name that says what it is. - git mv tests/integrate/tools -> tests/integrate/validation_tools. - Update every reference: Autotest.sh (both collector invocations), run_check.sh, Single_job.sh, docs/CONTRIBUTING.md (also fixing the stale ../tools relative path in the example), the Windows toolchain comment, and comments in props_common.sh. - Update the stale data-path comments in mathzone_add1_test.cpp from the long-gone tests/integrate/tools/PP_ORB to tests/PP_ORB. - tests/README: add a "How to validate an integrate test case" section explaining result.out/result.ref, the modular props_*.sh collectors, and the three validation entry points (Autotest.sh, Single_job.sh, catch_properties.sh + diff), and fix the stale Single.sh path. The unrelated ../tools/get_sum_*.py calls in props_deepks.sh are untouched on purpose: from a case CWD they resolve to tests/09_DeePKS/tools, a different directory. Verified: bash -n on all shell files; no integrate/tools references remain; collector and standalone DeePKS outputs are identical before and after the rename on the reference cases.
The split moved the deepks collector from a separate `bash` invocation (without -e) into the `bash -e` catch_properties.sh process, so helper commands that legitimately return nonzero now abort the whole collection: - process_npy step extraction: force/stress multi-mode files (ftot.npy, stot.npy) have no e<step> suffix, so `grep -oP 'e\d+'` returns 1 and killed the collector before the deepks_*_elec keys were written; every deepks_out_freq_elec case was reported as fatal. - the deepks_v_delta < 0 branch: CompareFile.py exits 1 when files differ; capture the raw status in named variables so it is still recorded instead of aborting. Verified with build_max_para_test/abacus_max_para (v3.11.0-beta10): all 31 cases in tests/09_DeePKS pass, 284 key checks OK, including 25_NO_GO_deepks_out_freq_elec and 26_NO_KP_deepks_out_freq_elec.
Running the integrate tests required exporting ABACUS_EXE in every new shell (or editing tracked files, which risks committing a local absolute path). Autotest.sh now sources integrate/general_info.local at startup when it exists; the file is gitignored and may set the 'abacus' variable (or export ABACUS_EXE). Effective priority: -a flag > ABACUS_EXE environment > general_info.local > 'abacus' from PATH; behavior is unchanged when the file is absent. Document the option in tests/README. Verified: with the local file present and ABACUS_EXE unset, Autotest.sh resolves the override and case 25_NO_GO_deepks_out_freq_elec passes; env override, -a override and no-file fallback behave as documented.
Rename tests/README to tests/README.md (history preserved via git mv) and reformat it as Markdown: proper headings, a table for the folder overview and fenced code blocks for the commands. Fix a few typos (multple, acripts, integrte, Cmake) and update the self-reference to README.md. No other file references tests/README, so no further updates are needed.
mohanchen
changed the title
mohanchen
changed the title
Critsium-xy
reviewed
Oct 8, 2026
Critsium-xy
left a comment
Critsium-xy
left a comment
Collaborator
There was a problem hiding this comment.
tests/integrate/CMakeLists.txt (not in the diff): integrated_test / integrated_test_with_asan still run Autotest.sh in tests/integrate with the default cases_file=CASES_CPU.txt, which this PR deletes. Every CTest run now logs Please specify test cases file by -f option. and cat: CASES_CPU.txt: No such file or directory, and still passes with 0 cases (the exit 1 at Autotest.sh:407 runs in a subshell). Please keep the empty files, or remove/repoint these tests.
- catch_properties.sh: run the DeePKS collector in a subshell with errexit disabled, restoring the pre-split behavior where a failing get_sum_*.py or a missing deepks_desc.dat leaves an empty value instead of aborting the whole collection under `bash -e`. - run_check.sh: substitute only the ABACUS_EXE placeholders instead of `eval`, quote the executable path, and check it with `command -v` so the documented fallback to `abacus` from PATH works. - Single_job.sh/run_check.sh: `debug` now copies the whole validation_tools/ directory into the case directory and collects with that copy, since catch_properties.sh sources its modules from its own directory. - Autotest.sh: save ABACUS_EXE before sourcing general_info.local so the environment keeps priority, and clear `abacus` so a stray environment variable of that name is not picked up. - Restore the empty tests/integrate/CASES_CPU.txt/CASES_GPU.txt used by the integrated_test CTest entries. - props_common.sh: parse out_alllog in props_init() and correct the module/sourcing contract comments. - README: run Autotest.sh from a category directory; describe debug mode. - general_info: drop trailing space after EXEC. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Critsium-xy
approved these changes
Oct 9, 2026
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
2 participants
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.
-Fixes #8091
Background
tests/integrate/tools/catch_properties.shwas a 1002-line monolithicscript that collected every kind of result line (energies, forces,
matrices, cubes, ML, DeePKS, TDDFT, ...) into
result.outfor theintegrate tests. Locating or extending a check meant scrolling one huge
file, the helper directory name (
tools) did not say what it was for,and pointing the tests at a local ABACUS build required editing tracked
files or exporting
ABACUS_EXEin every new shell.Changes
Split the collector into modules (cc4a937..0c9d1c1):
props_common.sh: shared helpers (sum_file,get_input_key_value,record_compare_result), absolute tool paths (PROPS_TOOLS_DIR),props_init()(INPUT switch parsing, result-file reset) andprops_finalize()(total time).props_basic.sh,props_mat.sh,props_cube.sh,props_ml.sh,props_tddft.sh,props_deepks.sh: one module per result family,each defining
run_<category>_props()hooks.catch_properties.shshrinks to a 101-line orchestrator that callsthe hooks in the original order; positional
post_*hooks keep thehistorical output-line ordering byte-for-byte.
catch_deepks_properties.shbecomes a thin standalone entry; itintentionally does not call
props_init()so it keeps appending toa caller-supplied result file.
calculationread, an unreachableout_dmbranch, and the unusedcompare-wfcsubcommand ofcube_tool.py.../../integrate/toolsrelative paths replaced by$PROPS_TOOLS_DIR.Rename
tests/integrate/tools->tests/integrate/validation_toolsvia
git mv(history preserved); update all references (Autotest.sh,Single_job.sh,run_check.sh,docs/CONTRIBUTING.md,toolchain/toolchain_windows.sh, and a stale comment insource/source_base/test/mathzone_add1_test.cpp). The../tools/paths inside the DeePKS collector resolve relative to the case CWD
(
tests/09_DeePKS/<case>->tests/09_DeePKS/tools/) and areintentionally unchanged.
Executable selection for test runs:
Autotest.shandgeneral_info(expanded byrun_check.sh) sharethe
ABACUS_EXEenvironment variable (d14c90b).Autotest.shsources an untracked, gitignoredintegrate/general_info.localwhen present; priority is-aflag >ABACUS_EXE> local file > PATH, so a machine-specificbuild path survives new shells without touching tracked files
(298eda7).
Fix a regression introduced by the split (5820e4f): the DeePKS
collector previously ran as a separate
bashinvocation without-e; in-process underbash -ethe filename step extractiongrep -oP 'e\d+'(force/stress files carry noe<step>suffix) andCompareFile.py's nonzero exit on mismatch aborted the wholecollection, so every
deepks_out_freq_eleccase was reported asfatal. Both statuses are now tolerated explicitly, matching the
pre-split behavior.
Documentation:
tests/READMEgains a "How to validate anintegrate test case" section and is converted to
README.md(367dfff).
Verification
bash -npasses for all touched shell scripts.monolith on every case with existing products; a full scan over 384
such cases produced identical
result.outcontent and exit codes(modulo
totaltimeref, which is machine-dependent by design).tests/09_DeePKScategory (31 cases) withOMP_NUM_THREADS=1, 4 MPI ranks, executableabacus_max_para(ABACUS v3.11.0-beta10): all pass (284 key checks), including
25_NO_GO_deepks_out_freq_elecand26_NO_KP_deepks_out_freq_elec,which failed fatally before 5820e4f.
general_info.local: header checks for the four priority scenarios(local file used; env overrides it;
-aoverrides everything; absentfile falls back to PATH) plus an end-to-end single-case run with
ABACUS_EXEunset.integrate/toolspaths outside the intentionally kept case-relative../tools/in DeePKS. CI only invokesAutotest.sh, which is updated.docs/parameters.yamlanddocs/advanced/input_files/input-main.mdneed no update.