Fix #8091: Test: split catch_properties.sh into modular validation collectors by mohanchen · Pull Request #8090 · deepmodeling/abacus-develop

mohanchen · 2026-10-07T12:34:11Z

Background

tests/integrate/tools/catch_properties.sh was a 1002-line monolithic
script that collected every kind of result line (energies, forces,
matrices, cubes, ML, DeePKS, TDDFT, ...) into result.out for the
integrate 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_EXE in 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) and
  props_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.sh shrinks to a 101-line orchestrator that calls
  the hooks in the original order; positional post_* hooks keep the
  historical output-line ordering byte-for-byte.
- catch_deepks_properties.sh becomes a thin standalone entry; it
  intentionally does not call props_init() so it keeps appending to
  a caller-supplied result file.
- Dead code removed: a duplicate calculation read, an unreachable
  out_dm branch, and the unused compare-wfc subcommand of
  cube_tool.py.
- Hard-coded ../../integrate/tools relative paths replaced by
  $PROPS_TOOLS_DIR.
Rename tests/integrate/tools -> tests/integrate/validation_tools
via 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 in
source/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 are
intentionally unchanged.
Executable selection for test runs:
- Autotest.sh and general_info (expanded by run_check.sh) share
  the ABACUS_EXE environment variable (d14c90b).
- Autotest.sh sources an untracked, gitignored
  integrate/general_info.local when present; priority is
  -a flag > ABACUS_EXE > local file > PATH, so a machine-specific
  build 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 bash invocation without
-e; in-process under bash -e the filename step extraction
grep -oP 'e\d+' (force/stress files carry no e<step> suffix) and
CompareFile.py's nonzero exit on mismatch aborted the whole
collection, so every deepks_out_freq_elec case was reported as
fatal. Both statuses are now tolerated explicitly, matching the
pre-split behavior.
Documentation: tests/README gains a "How to validate an
integrate test case" section and is converted to README.md
(367dfff).

Verification

bash -n passes for all touched shell scripts.
Split equivalence: the split collectors were run against the pre-split
monolith on every case with existing products; a full scan over 384
such cases produced identical result.out content and exit codes
(modulo totaltimeref, which is machine-dependent by design).
DeePKS regression: full tests/09_DeePKS category (31 cases) with
OMP_NUM_THREADS=1, 4 MPI ranks, executable abacus_max_para
(ABACUS v3.11.0-beta10): all pass (284 key checks), including
25_NO_GO_deepks_out_freq_elec and 26_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; -a overrides everything; absent
file falls back to PATH) plus an end-to-end single-case run with
ABACUS_EXE unset.
Rename: repo-wide search finds no remaining references to the old
integrate/tools paths outside the intentionally kept case-relative
../tools/ in DeePKS. CI only invokes Autotest.sh, which is updated.
No INPUT parameter behavior is changed, so docs/parameters.yaml and
docs/advanced/input_files/input-main.md need no update.

…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.

…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.

Critsium-xy

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.

Critsium-xy · 2026-10-08T04:32:18Z

+
        # Calculate value based on operation
        if [ "$op" = "abs" ]; then
            val=$(python3 ../tools/get_sum_abs.py "$file")


Behavior change: the DeePKS collector used to run as a separate bash process without -e; it now runs inside catch_properties.sh (bash -e). Only the grep -oP and CompareFile.py calls were made tolerant. If get_sum_*.py fails here (lines 75/77/79, e.g. corrupt .npy) or OUT.autotest/deepks_desc.dat is missing for the sed at line 153, the whole collector now aborts. Previously empty/zero values were written and caught by the threshold check; now the case reports Fatal Error in catch_properties.sh and all later sections (symmetry, TDDFT, RDMFT, alllog, totaltimeref) are missing. Suggest guarding these calls (e.g. || val=0) or running this module in a subshell without -e.

Critsium-xy · 2026-10-08T04:32:18Z

 exec_path=`grep EXEC $GENERAL_INFO_FILE | awk '{printf $2}'`
+exec_path=$(eval echo "$exec_path")

 test -e $exec_path || echo "Error! ABACUS path was wrong!!"


With the new default EXEC ${ABACUS_EXE:-abacus}, exec_path becomes abacus when ABACUS_EXE is unset. test -e abacus checks for a file in the case directory, not on PATH, so this prints Error! ABACUS path was wrong!! and exits 0 without running anything (including Single_job.sh ref). So the PATH fallback described in tests/README.md (lines 66-69) does not work for Single_job.sh. Suggest command -v "$exec_path" >/dev/null.

Critsium-xy · 2026-10-08T04:32:18Z

+# expanded from the environment (e.g. "EXEC ${ABACUS_EXE}"), so a single
+# env var can override the executable without editing general_info.
 exec_path=`grep EXEC $GENERAL_INFO_FILE | awk '{printf $2}'`
+exec_path=$(eval echo "$exec_path")


eval echo "$exec_path" executes arbitrary shell from the EXEC field ($(...), backticks), and the result is used unquoted below (test -e $exec_path, mpirun ... $exec_path), so a path with spaces breaks. Consider expanding only the known placeholder (e.g. substitute ${ABACUS_EXE:-abacus} explicitly) and quoting "$exec_path".

Critsium-xy · 2026-10-08T04:32:18Z

+1. Whole category (what ctest/CI do), from `tests/integrate`:
+
+   ```bash
+   bash Autotest.sh -n 4


There are no cases in tests/integrate and this PR deletes tests/integrate/CASES_CPU.txt, so bash Autotest.sh -n 4 (and -r 035_PW_15_SO below) from tests/integrate runs 0 cases and still exits 0 with [ PASSED ] 0 test cases passed. This should be run from a category directory, e.g. cd tests/01_PW && bash ../integrate/Autotest.sh -n 4.

Critsium-xy · 2026-10-08T04:32:18Z

 #!/bin/bash

-TOOLS_DIR="../../integrate/tools/"
+TOOLS_DIR="../../integrate/validation_tools/"


debug mode (lines 18-20) copies catch_properties.sh into the case directory, but the new entry script resolves PROPS_SCRIPT_DIR from its own location and sources props_*.sh from there, so the copy fails (props_common.sh not found, then props_init/run_*_props: command not found). Also, the copied run_check.sh still calls the fixed CATCH_SCRIPT path, so local edits are never used. Either copy all props_*.sh + helpers, or drop/redocument debug (README lines 179-180).

Critsium-xy · 2026-10-08T04:32:18Z

+# Optional local override file, not tracked by git (see .gitignore). When
+# present it is sourced first and may set the executable, e.g.:
+#     abacus=/home/me/abacus/build/abacus
+# or export ABACUS_EXE=/home/me/abacus/build/abacus


general_info.local is sourced before ${ABACUS_EXE:-...} is evaluated, so following this suggestion (export ABACUS_EXE=... in the file) overrides an ABACUS_EXE already set in the environment, contradicting the stated priority ABACUS_EXE environment > general_info.local. Also, line 20 now picks up an environment variable named abacus, which the old hard-coded abacus=abacus ignored. Suggest removing the export ABACUS_EXE suggestion, or saving the env value before sourcing.

Critsium-xy · 2026-10-08T04:32:18Z

+run_deepks_props(){
+
 # Main script
 has_force=$(get_input_key_value "cal_force" "INPUT")


cal_force/cal_stress (and out_alllog in run_basic_props_post_rdmft) are re-read from INPUT here, although props_common.sh says props_init() parses every INPUT switch once into globals. Also, the props_common.sh header says each props_<category>.sh module sources it, but none does; the entry scripts do. Please either use the globals (and let catch_deepks_properties.sh call props_init) or update the props_common.sh comments.

Critsium-xy · 2026-10-08T04:32:18Z

@@ -1,3 +1,3 @@
-EXEC abacus
+EXEC ${ABACUS_EXE:-abacus} 


Trailing space after ${ABACUS_EXE:-abacus}. It only works because awk '{printf $2}' drops it; please remove.

abacus_fixer added 9 commits October 7, 2026 07:57

mohanchen added Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0 Tests/Examples Issues/PR related to unit tests and integrate tests labels Oct 7, 2026

abacus_fixer and others added 5 commits October 7, 2026 20:36

Merge branch 'develop' into 2026-10-07-a

3e75301

mohanchen changed the title ~~Test: split catch_properties.sh into modular property collectors~~ Test: split catch_properties.sh into modular validation collectors Oct 7, 2026

mohanchen changed the title ~~Test: split catch_properties.sh into modular validation collectors~~ Fix #8091: Test: split catch_properties.sh into modular validation collectors Oct 7, 2026

mohanchen requested a review from Critsium-xy October 7, 2026 14:11

Critsium-xy reviewed Oct 8, 2026

View reviewed changes

Navigation Menu

Repository navigation

Fix #8091: Test: split catch_properties.sh into modular validation collectors - #8090

Fix #8091: Test: split catch_properties.sh into modular validation collectors#8090
mohanchen wants to merge 14 commits into
deepmodeling:developfrom
mohanchen:2026-10-07-a

mohanchen commented Oct 7, 2026 •

edited

Loading

Uh oh!

Critsium-xy left a comment

Uh oh!

Critsium-xy Oct 8, 2026

Uh oh!

Critsium-xy Oct 8, 2026

Uh oh!

Critsium-xy Oct 8, 2026

Uh oh!

Critsium-xy Oct 8, 2026

Uh oh!

Critsium-xy Oct 8, 2026

Uh oh!

Critsium-xy Oct 8, 2026

Uh oh!

Critsium-xy Oct 8, 2026

Uh oh!

Critsium-xy Oct 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mohanchen commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Changes

Verification

Uh oh!

Critsium-xy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mohanchen commented Oct 7, 2026 •

edited

Loading