Skip to content

Commit 97697c2

Browse files
mohanchenabacus_fixerCritsium-xyclaude
authored
Fix #8091: Test: split catch_properties.sh into modular validation collectors (#8090)
* tests: select ABACUS executable via ABACUS_EXE and document test workflow 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. * tests: drop unused compare-wfc subcommand from cube_tool.py 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. * tests: extract props_common.sh shared library (step 1/7) 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. * tests: extract props_basic.sh collectors (step 2/7) 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. * tests: extract props_mat.sh matrix collectors (step 3/7) 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. * tests: extract props_cube.sh cube/wfc collectors (step 4/7) 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. * tests: extract props_ml.sh ML/RPA/LR/RDMFT collectors (step 5/7) 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. * tests: extract props_tddft.sh/props_deepks.sh (step 6/7) - 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. * tests: final cleanup of split property collectors (step 7/7) - 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. * tests: rename integrate/tools to validation_tools and document validation 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. * Test: tolerate nonzero helper status in deepks collector under bash -e 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. * Test: let Autotest.sh source an untracked general_info.local override 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. * Test: convert tests/README to Markdown 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. * Test: address review comments on split property collectors - 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> --------- Co-authored-by: abacus_fixer <mohanchen@pku.eud.cn> Co-authored-by: Critsium <tsfxwbbzxy@163.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 0836dce commit 97697c2

25 files changed

Lines changed: 1593 additions & 1336 deletions

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,4 @@ toolchain/abacus_env.sh
2929
.trae
3030
.codex
3131
compile_commands.json
32+
tests/integrate/general_info.local

‎docs/CONTRIBUTING.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ The integrate test is a test suite for testing the whole ABACUS package. The exa
396396
- For PW calculations, should set `pw_seed 1` in INPUT file to ensure the reproducibility of the test.
397397
3. Generate the reference results for the new test.
398398
- Run the new test with GNU compiler and 4 MPI processes 2 OpenMP threads. The command is `OMP_NUM_THREADS=2 mpirun -np 4 abacus > log.txt`.
399-
- Execute tests/integrate/tools/catch_properties.sh script to generate the reference results. At the new test directory, run `bash ../tools/catch_properties.sh result.ref`.
399+
- Execute tests/integrate/validation_tools/catch_properties.sh script to generate the reference results. At the new test directory, run `bash ../../integrate/validation_tools/catch_properties.sh result.ref`.
400400
A `result.ref` file may be like:
401401
```text
402402
etotref -3439.007931317310
@@ -410,7 +410,7 @@ The integrate test is a test suite for testing the whole ABACUS package. The exa
410410
if ! test -z "$has_band" && [ $has_band == 1 ]; then # if band is outputed, then check if the band is correct
411411
bandref=refBANDS_1.dat # this file should be prepared in new test directory
412412
bandcal=OUT.autotest/BANDS_1.dat # this file is generated by each run of test
413-
python3 ../tools/CompareFile.py $bandref $bandcal 8 # compare the new band file with the reference file
413+
python3 ../../integrate/validation_tools/CompareFile.py $bandref $bandcal 8 # compare the new band file with the reference file
414414
echo "CompareBand_pass $?" >>$1 # record the comparison result, $? is the return value of last command
415415
fi
416416
```

‎source/source_base/test/mathzone_add1_test.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ TEST_F(MathzoneAdd1Test, Constructor)
6969
/// first kind boundary condition: f'(0) = f'(n) = 0.0
7070
TEST_F(MathzoneAdd1Test, CubicSplineBoundary1)
7171
{
72-
// data from abacus/tests/integrate/tools/PP_ORB/Si_gga_8au_60Ry_2s2p1d.orb
72+
// data from abacus/tests/PP_ORB/Si_gga_8au_60Ry_2s2p1d.orb
7373
// data for d orbital of Si : L = 2, N = 0
7474
psi_in[0] = 0;
7575
psi_in[1] = -2.583946346740e-01;
@@ -232,7 +232,7 @@ TEST_F(MathzoneAdd1Test, expx)
232232
/// second kind boundary condition: f''(0) = f''(n) = 0.0
233233
TEST_F(MathzoneAdd1Test, CubicSplineBoundary2)
234234
{
235-
// data from abacus/tests/integrate/tools/PP_ORB/Si_gga_8au_60Ry_2s2p1d.orb
235+
// data from abacus/tests/PP_ORB/Si_gga_8au_60Ry_2s2p1d.orb
236236
// data for 1st p orbital of Si: L = 1, N= 0
237237
psi_in[0] = 0;
238238
psi_in[1] = 2.023466616834e-01;
@@ -279,7 +279,7 @@ TEST_F(MathzoneAdd1Test, CubicSplineBoundary2)
279279

280280
TEST_F(MathzoneAdd1Test, UniRadialF)
281281
{
282-
// data from abacus/tests/integrate/tools/PP_ORB/Si_gga_8au_60Ry_2s2p1d.orb
282+
// data from abacus/tests/PP_ORB/Si_gga_8au_60Ry_2s2p1d.orb
283283
// data for 1st p orbital of Si: L = 1, N= 0
284284
psi_in[0] = 0;
285285
psi_in[1] = 2.023466616834e-01;

‎tests/README‎

Lines changed: 0 additions & 151 deletions
This file was deleted.

0 commit comments

Comments
 (0)