Skip to content

Feature: rework out_app_flag as a three-mode output controller with unified output directories, append semantics, and provenance headers - #8159

Open
mohanchen wants to merge 65 commits into
deepmodeling:developfrom
mohanchen:2026-10-16-b
Open

mohanchen wants to merge 65 commits into
deepmodeling:developfrom
mohanchen:2026-10-16-b

Conversation

@mohanchen

@mohanchen mohanchen commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This PR reworks out_app_flag from a boolean into a three-mode integer switch
and, on top of that, reorganizes ionic-step outputs into unified directories,
fixes several MPI/MD output bugs, and adds machine-readable provenance headers
to user-facing output files.

out_app_flag semantics (bool -> int)

  • out_app_flag is now an int with three modes:
    • 0 (default): legacy per-step file names and layout (unchanged behavior).
    • 1: fixed file names; per-ionic-step records are appended to a single
      accumulated file (cube frames, PW/LCAO wavefunctions, band output, DOS/PDOS,
      DM(k), T(R)/R(R)/dH/dS sparse matrices, STRU, ...), each record carrying a
      one-based STEP: N provenance header.
    • 2: per-step files routed into unified subdirectories (STRU/ for
      structures, MAT/ for sparse matrices such as dmr/tr/rr/dh/ds/lx, etc.),
      created on demand.
  • INPUT documentation updated to describe the fixed-name behavior of mode 1.

Output renames and provenance headers

  • Orbital information output renamed from Orbital to nao.txt.
  • Band-energy-term outputs renamed (kinetic_out -> eband_kin,
    vhartree_out -> eband_hart, vpp_local_out -> eband_vl,
    vpp_nonlocal_out -> eband_vnl, vxc -> eband_vxc), with per-ionic-step
    provenance headers added to band output.
  • Band-decomposed XC output renamed; LIBRPA producer reference files relocated
    accordingly (manifests updated).
  • LCAO k-space matrix output parameters renamed, and misleading docs fixed.
  • Text LCAO wavefunction output gains a provenance header; the integration
    comparison tool skips comment headers.
  • Ionic-step indices are printed one-based in user-facing outputs (STEP: 1,
    matching the g{N} filename suffix convention), including binary H(R)/S(R)
    and sparse DMR writers; governance doc updated accordingly.

MD / relaxation structure output

  • STRU output directory and naming unified between relax and MD.
  • MD STRU numbering now starts from 1, consistent with relax.
  • md_restartfreq replaced by out_freq_ion + out_stru.
  • Legacy STRU directory layout is preserved for MD structure writing, and the
    MD restart STRU lookup broken by the rework is restored.

Bug fixes

  • Fix MPI crashes in write_dipole when out_dipole=1.
  • Keep all ranks on the FFT path in write_elf (fixes ELF output hangs/crashes).
  • Reuse the fixed dH/dS CSR filename in append mode and log every write.
  • Terminate the DM(k) matrix body with a newline.
  • Route remaining PW outputs through out_app_flag and drop the incorrect
    LCAO-only availability claim.
  • Use the real out_app_flag value for PW wavefunction output; use local
    cached directory variables in ctrl_output_fp.

Tests

  • New three-mode unit tests covering out_app_flag naming and append
    semantics, save_mat overwrite, and write_hsk mode-2 rerun semantics.
  • Sparse-matrix reference files and DMR writer test updated for one-based
    STEP headers; DM(k) test crashes fixed.
  • Integration reference files updated for renamed outputs
    (props_cube.sh, props_mat.sh, LIBRPA manifests, etc.).

Backward compatibility

  • Default behavior (out_app_flag=0) is unchanged; mode 1/2 are opt-in.
  • md_restartfreq is removed in favor of out_freq_ion + out_stru;
    existing inputs using md_restartfreq must migrate.
  • Several output file names changed as listed above; downstream parsers and
    LIBRPA producer consumers should use the updated manifests.

Verification

  • Unit tests: new/updated tests under source/source_io/test,
    source/source_lr, and module test directories (three-mode naming/append,
    sparse writers, STEP headers).
  • Integration tests: updated references in tests/01_PW, 02_NAO_Gamma,
    03_NAO_multik, 08_RI, plus validation tools (props_*.sh,
    check_librpa_producer.py, compare_hsr_binary.py).

abacus_fixer added 8 commits October 16, 2026 14:47
…unified output directories

- Change out_app_flag from bool to int with three states:
  0: no append, no subdirectory (overwrite)
  1: append to same file (default, backward compatible)
  2: write to unified subdirectories
- Add new global directory variables in System_para:
  global_psi_dir, global_chg_dir, global_pot_dir,
  global_elf_dir, global_dos_dir, global_dip_dir, global_mat_dir
- Initialize directories in set_global_dir():
  out_app_flag==2 -> OUT.suffix/{PSI,CHG,POT,ELF,DOS,DIP,MAT}/
  otherwise -> all point to global_out_dir (backward compatible)
- Map legacy WFC->PSI, matrix->MAT
- Update all out_app_flag usages from bool to int semantics
- Broadcast new directory variables via Parallel_Common::bcast_string

Issue: deepmodeling#8151
- ctrl_output_fp.cpp: route chg/tau to global_chg_dir, pot/potes to
  global_pot_dir, ELF to global_elf_dir, dipole to global_dip_dir
- ctrl_output_pw.cpp: route wfc to global_psi_dir, dos to global_dos_dir,
  ldos to global_dos_dir
- ctrl_scf_lcao.cpp: route dos to global_dos_dir
- ctrl_runner_lcao.cpp: route ldos to global_dos_dir
- read_input.cpp: create subdirectories (PSI/CHG/POT/ELF/DOS/DIP/MAT)
  when out_app_flag==2
- Update docs: out_app_flag semantics from bool to int (0/1/2)

Issue: deepmodeling#8151
When out_app_flag defaults to 1 (not 2), all new unified directories
(PSI, CHG, POT, ELF, DOS, DIP, MAT) map to global_out_dir for
backward compatibility. Update io_sys_var_test.cpp expectations:
- global_matrix_dir: "OUT.test/matrix/" -> "OUT.test/"
- Add assertions for all 7 new directory variables
- read_input.cpp: change !out_app_flag to out_app_flag == 0 for
  matrix/WFC legacy directory decisions
- Update three unit tests to use int out_app_flag (0/1) instead of
  bool (false/true), matching the new function signatures:
  test_cal_dos.cpp, test_write_dos_pw.cpp, write_wfc_nao_test.cpp
- Update two comments to describe 0/1 semantics
Replace repeated PARAM.globalv.global_*_dir accesses with the local
const variables already declared at function top, eliminating the
net +4 PARAM global dependency introduced by the path routing change.
With out_app_flag changed from bool to int, the legacy WFC/ subdirectory
is no longer created when out_app_flag=0 (now maps to global_out_dir).
Update the multi-k wfc reference path accordingly.
- read_input.cpp: only create PSI/CHG/POT/ELF/DOS/DIP/MAT subdirectories
  when the corresponding output flag is enabled (out_app_flag=2)
- ctrl_scf_lcao.cpp: route write_dmk and angular momentum calculator
  output to global_mat_dir instead of global_out_dir
- hsr_legacy.cpp: route tr_nao.csr to global_matrix_dir
- pos_op_writer.cpp: route rr_nao.txt to global_matrix_dir
@mohanchen mohanchen added Input&Output Suitable for coders without knowing too many DFT details Refactor Refactor ABACUS codes labels Oct 16, 2026
@mohanchen

Copy link
Copy Markdown
Collaborator Author

@ABACUS-PR-BOT please help review the PR

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review of 00ece15 with independent verification. Five implementation issues and three non-blocking suggestions are noted inline. Whitespace checks passed; the pinned governance check reported warnings only. No build or runtime tests were run, and CI diagnostics were unavailable.

Comment thread source/source_io/module_parameter/read_set_globalv.cpp
Comment thread source/source_io/module_parameter/read_input.cpp Outdated
Comment thread source/source_io/module_ctrl/ctrl_scf_lcao.cpp
Comment thread source/source_io/module_parameter/read_inp_out.cpp Outdated
Comment thread source/source_io/module_hs/hsk_writer.cpp
Comment thread source/source_io/module_parameter/read_inp_out.cpp Outdated
Comment thread source/source_io/test_serial/io_sys_var_test.cpp
Comment thread source/source_io/module_parameter/read_input.cpp Outdated
abacus_fixer and others added 18 commits October 18, 2026 08:10
- Remove md_no_append ternary in hsr_legacy.cpp, pos_op_writer.cpp,
  hterm_writer.cpp, dhs_sparse_writer.cpp: use global_matrix_dir
  unconditionally (it already equals global_out_dir when
  out_app_flag != 2, so no behavior change for legacy modes)
- write_dmr: add dmr_dir parameter, route DM(R) to MAT instead of
  hardcoded global_out_dir; drop parameter.h include
- read_input.cpp: add out_dmr/out_mat_l/out_mat_dh/out_mat_ds to
  MAT subdirectory creation flags

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- global_stru_dir now follows out_app_flag: STRU/ for flag 2,
  global_out_dir otherwise (consistent with MAT/PSI/CHG pattern)
- relax_driver: route STRU_NOW/STRU{step}/STRU_FINAL through
  global_stru_dir instead of global_out_dir
- run_md: drop _MD suffix, use STRU{step} naming consistent with
  relax
- global_file: remove unconditional STRU dir creation from
  make_dir_out; remove global_stru_dir parameter
- read_input: create STRU dir on demand (out_stru>0 or MD)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- run_md: use out_freq_ion (with md_restartfreq fallback) as the
  effective ionic-step output frequency for both STRU and restart
- run_md: gate STRU output on out_stru > 0, consistent with relax
- read_input: simplify STRU dir creation to out_stru > 0
- read_inp_out: update out_stru description to cover MD
- read_input_item_md: mark md_restartfreq as deprecated

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- commit 3a38485 made global_stru_dir follow out_app_flag (default 1
  maps it to the output dir), but the MD restart lookup still probed
  STRU_MD_{istep} there, so restart cases whose fixtures live in
  OUT.suffix/STRU/ (e.g. tests/04_FF/14_LJ_rescale_v) failed to start
- search the current global_stru_dir first, then fall back to the
  legacy OUT.suffix/STRU/ layout; accept both the new STRU{istep+1}
  naming (1-based, unified with relax) and the legacy STRU_MD_{istep}
- quit with a clear diagnostic when no restart structure file exists
- add unit test covering both naming schemes in a user read_file_dir
- add PSI/ candidate in read_wfc_nao so RT-TDDFT restart finds mode-2 snapshots
- route H(k) term outputs to MAT directory; add out_hsr_npz to MAT dir creation
  predicate; narrow DOS dir creation predicate to out_dos/out_ldos
- pass boolean append to save_mat in write_hk_common (was int out_app_flag)
- update out_app_flag/out_wfc_lcao INPUT metadata and position_matrix docs
- add mode-2 directory mapping and PSI layout roundtrip unit tests
Conflict resolutions:
- read_input_test.cpp: keep both added tests (OutAppFlagParsing ours,
  ExplicitNoccSurvivesUnresolvedElectronNumber upstream)
- relax_driver.cpp: identical condition; keep ours comment (md STRU
  output now handled by run_md.cpp, md_restartfreq removed)

Post-merge verification: make -j 30 in build_max_para_test, 100%
Built target abacus_max_para, 0 errors.
In append mode (out_app_flag=1), chg/tau/pot/potes/ELF cube outputs
now reuse a fixed filename (e.g. chgs1.cube, elftot.cube) so every
step overwrites the previous snapshot instead of accumulating
g{N}-suffixed files. Modes 0 and 2 keep the per-step g{N} suffix.
Add an explicit out_app_flag parameter to ModuleIO::write_dmk. In
append mode the geometry-step index is dropped from the filename
(istep_file = -1) so every ionic step overwrites the previous
snapshot, matching the fixed name that read_dmk already expects;
the file body still records the real ionic step.
Drop the g{N} suffix from tr_nao.csr and rr_nao.txt in append mode.
The filenames were changing every ionic step, which defeated the
existing MD append branches in save_sparse and the rr assemble path;
with fixed names those branches now accumulate all steps as designed.
Replace the hardcoded out_app_flag=0 in write_wfc_pw (a 2025-05-17
TODO) with an explicit parameter wired from inp.out_app_flag. In the
default append mode the PW wavefunction files now use fixed names
(wfk1_pw.dat), which finally matches what the restart reader
read_wf2rho_pw looks for; modes 0 and 2 keep the g{N} suffix.
write_chg_init/write_pot_init now keep the fixed _ini.cube names in
append mode so each snapshot overwrites the previous one, instead of
accumulating chgs1g{N}_ini.cube / potg{N}_ini.cube files. These
initial-density cubes are write-only snapshots; no reader consumes
them, so no restart path is affected.
Clarify that mode 1 reuses fixed filenames across steps (text records
append, cube snapshots overwrite), fix the remaining Boolean wording
in out_hsk, and note that out_chg=2/out_pot=3 initial-density cubes
omit the geometry-step index in append mode.
The fixed filename alone still truncated the file every ionic step.
Add a calculation parameter and open the stream with ios::app under
the same condition as open_sparse_file (calculation=="md" &&
out_app_flag==1 && istep>=0), so each MD step appends a new record
while other calculations start fresh.
Rule 22: user-facing output files print ionic steps starting from 1,
matching the g{N} filename suffix convention; zero-based counters such
as "STEP: 0" are not allowed.
Naming (mode 1 fixed name, modes 0/2 keep g{N}):
- DOS (LCAO+PW) and PDOS honor out_app_flag for the g{N} suffix
- lx/ly/lz matrices: Angmom_op::calculate takes out_app_flag

Append (relax now appends like MD; was calculation=="md" only):
- open_sparse_file, rr/dH/dS writers, DM(k): condition relaxed to
  (out_app_flag == 1) && (istep >= 0); SCF passes istep=-1, unaffected
- PDOS and lx/ly/lz append per ionic step in mode 1
- DM(k) drops the calculation parameter again (no longer used)

Step indexing (governance rule 22, one-based):
- STEP: headers of sparse text files and the binary CSR step field
  now start from 1 (hs_sparse_io, pos_op_writer, dhs_sparse_writer,
  hsr_writer)

Build: make -j 30 verified by developer.
Instead of numbered STRU{N} files, out_app_flag=1 now writes one
fixed-name STRU file that accumulates one structure block per written
step, for both relax and MD:

- unitcell::print_stru_file and relax_stru_io::write_stru take a new
  append flag; relax writes "# Ionic Step N" headers (one-based,
  governance rule 22) via the existing build_stru_header
- mdcell::print_stru_file takes frame_comment + append; MD frames are
  prefixed with "# MD step N". The MPI path broadcasts a base offset
  (lseek SEEK_END) and keeps absolute pwrite offsets; O_APPEND is
  deliberately avoided since Linux pwrite ignores the offset with it
- CIF output keeps single-frame semantics: fixed-name STRU.cif is
  overwritten each written step
- modes 0/2 keep the numbered STRU{N} files unchanged
- CLI help of out_stru/out_app_flag documents the mode-1 behavior

Build: make -j 30 verified by developer.
The DM(k) writer emitted the row newline only before each group of
elements, so the file ended without a trailing newline. In append mode
(out_app_flag == 1) the next " --- Ionic Step N ---" record was glued to
the last value of the previous step. Add a trailing newline before
closing the file and cover it with a regression assertion.
abacus_fixer and others added 10 commits October 21, 2026 16:31
Two bugs in source/source_io/module_dipole/write_dipole.cpp caused
out_dipole=1 to abort under MPI:

1. Validation check rejected rhopw->nplane == 0, but nplane is a
   per-rank local count (number of z-planes owned by the current
   process). Some ranks legitimately hold zero z-planes under z-axis
   MPI decomposition; hitting WARNING_QUIT there made those ranks
   exit with code 1 while the others waited forever.

2. The dipole data line was written to 'ofs' on every rank, but the
   file is only opened on rank 0. Non-root ranks wrote to an
   unopened stream, which is undefined behaviour and corrupts output
   when it doesn't crash outright.

Fixes:
- Remove rhopw->nplane == 0 from the guard; keep only global
  dimensions nx/ny/nz/nxyz, which truly must be positive.
- Wrap the ofs << ... write inside an if (GlobalV::MY_RANK == 0)
  block, consistent with the existing file-open and file-close guards.
A rank can own zero real-space grid points (nrxx == 0) when the FFT grid
is decomposed across more processes than it has z-slabs. The asserts in
write_elf rejected that legitimate empty partition. Dropping the asserts
alone is not enough if the FFT body is skipped: real2recip/recip2real go
through PW_Basis::gatherp_scatters, which performs an MPI_Alltoallv that
requires every rank in POOL_WORLD to participate, so a rank that returns
early deadlocks the grid-owning ranks inside Alltoallv.

Run the same FFT path unconditionally on every rank (the per-grid-point
loops simply iterate zero times when nrxx == 0) and keep every rank in the
write_vdata_palgrid output section for the collective MPI_Gatherv in
Parallel_Grid::reduce. Add regression tests for the zero-length container
and zero-iteration invariants.

Verified with examples/17_relax/04_relax_with_output_lcao (out_elf 1):
OMP_NUM_THREADS=4 mpirun -np 4 abacus_max_para finishes in 4 s and writes
elfs1.cube, elfs2.cube and elftot.cube; MODULE_IO_write_elf_logic_test
passes 14/14.

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com>
The 'Orbital' file only describes the NAO basis orbitals of the cell
(index, species, l, m, zeta, symmetry name) and is consumed by the
projected-band and Mulliken outputs. Rename it to 'nao.txt' so the
name states the basis type and does not collide conceptually with
plane-wave outputs.

Update both writers, the affected unit tests, the pband integration
comparison script and its golden reference, and the Mulliken docs.
wfk*_nao.txt now carries the same three-line header as bands1.txt
(version, timestamp, ionic step with spin tag) before the data block,
with one header per frame in append mode. The reader skips leading '#'
comment lines on the ASCII path only; the binary record layout is
untouched. Downstream Python consumers of the text format filter '#'
lines so they tolerate the new header.

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Align test_lr_util.cpp with governance rule 22: hs_sparse_io writes
STEP: max(istep,0)+1, and LR_Util::save_HR passes istep = 0, so the
header is "STEP: 1". Fixes MODULE_LR_dm_regressions and
MODULE_LR_sparse_rank_keys.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
With out_app_flag = 1 the multi-step writers reuse a fixed filename, so
write_cube truncated the previous snapshot and only the last written step
survived in chg/tau/pot/elf cube files. Text outputs already append records
under the same flag, and out_wfc_lcao already truncates on the first step and
appends afterwards, so cube output was the remaining inconsistency.

Pass an explicit append flag down the writer chain instead of reading
PARAM inside the writers:
- write_cube / write_vdata_palgrid take a new append parameter and open the
  file with std::ios::app when it is set, otherwise truncate;
- write_elf and write_elecstat_pot forward it, and write_chg_init /
  write_pot_init compute it from inp.out_app_flag;
- ctrl_output_fp uses append = (out_app_flag == 1 && istep > 0), mirroring
  the out_wfc_lcao rule: the first written ionic step truncates, later
  written steps append one complete frame whose first comment line is
  Ionic_Step {#}, one-based. Single-point SCF/NSCF stays at istep == 0, so it
  keeps overwriting rather than stacking on a stale file from a rerun.
- every other call site (LDOS, LCAO/PW wavefunction cubes, partial charge,
  LR-TDDFT density, exciton density, DM2rho) passes false explicitly to keep
  its existing truncate behaviour.

Update the out_app_flag and out_elf help text, the output specification, and
add CubeIOTest.WriteCubeAppendFrames covering truncate-then-append-then-
truncate frame counting and the trailing newline contract.

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com>
Rename vpp_local_out.dat, vpp_nonlocal_out.dat, vhartree_out.dat and
kinetic_out.dat to eband_vl.txt, eband_vnl.txt, eband_hart.txt and
eband_kin.txt, and prefix each file with version, timestamp, ionic step
and total energy, expectation-value formula, and unit comments. Write
the files on rank 0 only to avoid concurrent writes in MPI runs.
abacus_fixer added 4 commits October 22, 2026 09:00
…write

In out_app_flag=1 the previous code still generated a g{istep+1} suffixed
filename per ionic step, so the supposed "append" mode actually wrote a new
file each step and never accumulated frames.

- Generate the fixed dhrxs1_nao.csr / dsrxs1_nao.csr filename when
  out_app_flag == 1, matching the established one-based naming used by
  other NAO matrix writers.
- Open the file in truncate mode on the first written step (istep == 0)
  and append on subsequent steps.
- Always print the "Write ... to file:" line to the running log, including
  appended steps and binary output, so every written file is traceable.
- Add DhsSparseWriter.AppendModeReusesFixedFilename unit test covering
  modes 1 and 2.

Verification: ctest -V -R dhs_sparse_writer_test (4/4 pass),
make -j 30 abacus_max_para links cleanly.
Since v3.11 the text LCAO wavefunction writer prepends a provenance
header ("# ABACUS version:", "# Written at ...", "# IONIC STEP ..."),
and the multi-k awk pass forwards those comment lines into *_mod.txt.
The reference files predate the header and the "# Written at" timestamp
is run-dependent, so the wfc comparison always failed.

Strip comment lines from both the calculated and reference files before
invoking CompareFile.py, mirroring the existing header filtering in
props_mat.sh. Reference files are left unchanged.

Verified by rerunning scf_out_wf (gamma), 03_NAO_multik/scf_out_wf,
03_NAO_multik/md_out_wf and 05_rtTDDFT/02_NO_CH_OW_TDDFT with the
current build: all wfc comparisons pass (exit 0) after filtering.
…ference files

Rename the band-decomposed XC energy output from vxc.txt to eband_vxc.txt
for consistency with the other eband_* term files (eband_kin/vl/vnl/hart),
and rename the INPUT parameter out_eband_terms to out_eband.

- write_Vxc now always emits eband_vxc.txt (with provenance header) for
  either out_mat_xc or out_eband; the MO-basis vxc_nao.txt matrices remain
  gated by out_mat_xc alone.
- Pass ionic step and total energy into write_Vxc for the eband header.
- write_eband_terms no longer writes the XC term (handled by write_Vxc) and
  drops the now-unused EXX/DRANK arguments.
- Update integration cases (scf_out_hxc, scf_out_xc_multik) and LIBRPA
  producer manifests; reference files renamed via git mv (vxc.ref ->
  eband_vxc.ref, vxc.txt -> eband_vxc.txt).
- Move LIBRPA producer reference files out of the .gitignore'd
  producer_reference/OUT.autotest/ directory into producer_reference/
  (flattened), so new reference snapshots no longer need git add -f.
- compare scripts filter comment/header lines before diffing eband files.

Verification:
- Grouped and applied via patch-based hunk split (backup in
  /tmp/commit_split_backup). Not recompiled here; eband output paths and
  reference resolution verified by inspection of props_mat.sh,
  check_librpa_producer.py and the updated manifests.
…ding docs

The out_mat_h_* family of INPUT parameters printed k-space matrices in NAO
text format, but their docs falsely claimed R-space CSR output (which is
gated by WriteHParams::also_hR, always false upstream). Fix the docs to
describe the actual k-space output, and rename the parameters to match the
established *k suffix convention.

- Doc fix: out_mat_h_t/vnl/vl/vh/vxc/exx descriptions now state k-space
  NAO text output instead of the bogus "R in CSR format".
- out_mat_h_t is a legacy alias of out_mat_tk (the dedicated write_h_t
  path duplicated the T(k) output and is removed).
- Rename: out_mat_h_vnl->out_mat_vnlk, out_mat_h_vl->out_mat_vlk,
  out_mat_h_vh->out_mat_vhk, out_mat_h_vxc->out_mat_vxck,
  out_mat_h_exx->out_mat_exxk. Old names remain as legacy aliases resolved
  during input normalization (new name wins; legacy value maps over).
- Update MAT/ directory creation condition and ctrl_scf_lcao to the new
  canonical names.
- Add HkComponentAliases serial test covering alias mapping and
  new-name-wins priority.

Verification: compiled with make -j 30; HkComponentAliases, HsOutputAliases
and the full MODULE_IO ctest label (54 tests) pass; code_quality_score is
unchanged from the pre-change baseline (remaining debt is pre-existing).
@mohanchen mohanchen changed the title Unify output directory structure with out_app_flag=2 Feature: rework out_app_flag as a three-mode output controller with unified output directories, append semantics, and provenance headers Oct 23, 2026
abacus_fixer added 3 commits October 23, 2026 15:21
The out_app_flag=2 structure subdirectory shared the name of the STRU
input file, which is confusing in docs, scripts, and on case-insensitive
filesystems. Rename it to STRUC/ and update the unit test, INPUT help
text, and comments accordingly. MD restart still recognizes structure
files written into the legacy STRU/ subdirectory by previous releases.
Under grid-parallel MPI distribution a rank may own no real-space grid
points (rho_basis->nrxx == 0). write_h_vh then builds an empty matrix
v_h(nspin, 0) and unconditionally dereferences &v_h(ispin, 0), tripping
the matrix.h `ic < nc` assertion and aborting the run when
out_mat_h_vh / out_mat_h 2 / out_mat_vhks_k is enabled. The SCF path
itself is unaffected because it never indexes the matrix this way.

Keep setup_veff_hcontainer so the HContainer sparsity stays consistent
for the subsequent folding_HR/save_mat, and only skip the gint
potential integral when there is no local potential to integrate.
Same class of bug as the preceding Hartree fix: when a grid-parallel
rank owns no real-space grid points (nrxx == 0), write_h_vl receives a
null v_local from pot->get_fixed_v() and write_h_vxc gets an empty
v_xc matrix from XC_Functional::v_xc; both then index the potential
unconditionally. Keep setup_veff_hcontainer for a consistent HContainer
sparsity and only skip the gint integral when there is no local
potential on this rank.

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static incremental review of 964455d with independent verification. Source inspection confirms the earlier Boolean aliases, indexed root/PSI/WFC lookup, matrix routing, and unused-directory fixes (PR8159-F1, PR8159-F2, PR8159-F4, PR8159-N3). Residual issues and related follow-ups use their existing discussions; the new indexed cumulative-WFC concern is separate from the fixed lookup issue.

New findings and labeled non-blocking suggestions are inline. Whitespace checks passed; the pinned governance checker reported warnings only. No build, runtime, generated-doc or MPI test was run; exact-head CI diagnostics were unavailable.

? "# MD step " + std::to_string(stru_step) + "\n\n"
: "";
std::stringstream file;
file << stru_out_dir << "STRU";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep default MD output restartable

With default out_app_flag=1, a fresh MD run writes only accumulated OUT.suffix/STRU alongside Restart_md.txt. find_md_restart_stru searches STRU{step+1} and STRU_MD_{step}, so the next md_restart run cannot find this checkpoint. Please retain a separate restart snapshot or select the matching accumulated frame; simply accepting its first frame would not restore the saved step.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cumulative-file fallback fixes the normal lookup, but it selects the last frame without checking Restart_md.txt. run_md writes the structure before updating that checkpoint, so an interruption between those writes can resume an older step/thermostat state using newer geometry and velocities. Please select or validate the expected one-based frame istep+1 and test a later complete frame; this remaining concern is conditional P2, rather than the original unconditional restart failure.

gamma_only,
istep));
}
const int append_app_flag = 1;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Reject the wrong frame for indexed TDDFT restart

For a positive TDDFT restart step, this fallback accepts an unindexed accumulated WFC, but read_one_file always loads its first record. The legacy STRU-directory path makes this reachable while the field manager has already restored the later time step. Please reject this fallback for positive indexed restarts until the requested frame can be selected and validated; ordinary first-frame initialization can remain separate.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The positive-step restart guard is fixed, but it now also rejects ordinary TDDFT initialization from fixed-name SCF wavefunctions. Non-restart TDDFT sets restart_step_ to 0 and passes it here, while the unindexed fallback is now restricted to negative steps. Please preserve the ordinary step-zero initialization path (or pass an explicit initialization sentinel) while retaining positive-step rejection.

// In append mode (out_app_flag == 1) reuse the fixed filename and append
// each ionic step; single-point SCF passes istep = -1 and truncates.
const int istep_file = (out_app_flag == 1) ? -1 : istep;
const bool append_step = (out_app_flag == 1) && (istep >= 0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Start a fresh accumulation at the first new-run frame

This appends even at the first step of a new run, so rerunning mode 1 in an existing OUT directory retains previous PDOS records. The new DM(k) and STRU accumulators have the same first-step condition; selected L and positive-cadence multi-k relaxation T(R)/r(R) output also gained this behavior. Please separate append-this-frame from mode selection, truncate at a fresh first frame, and preserve append for genuine continuation.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fresh-run reset remains partial: MD step 0 now makes append_stru false, which also selects STRU1, so it never truncates the old fixed-name STRU and later frames append to the previous run. Separately, a real MD restart still passes local step 0 through the electronic solver, so the new >0 tests truncate existing PDOS/DM(k) history. Please separate fixed-name selection from append state and propagate an explicit fresh-run/continuation decision through these callers. The earlier bounded L/TR/RR/relax follow-ups in this thread remain unchanged.

// when out_app_flag == 1, so the file accumulates one frame
// per ionic step instead of being rewritten each step.
const bool append_frame = (istep > 0 && out_app_flag == 1);
if (!append_frame)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Separate PW output time from filename indices

With out_freq_ion=0, ctrl_iter_pw passes istep_in=-1 on every converged ionic step, so this new mode-1 append check always truncates the file. Updated PW help explicitly promises later-step accumulation in this configuration. Please pass the real output step separately from the filename sentinel and keep the electronic-iteration frame policy explicit for positive-frequency output.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filename-sentinel issue is fixed in the current source: the caller now passes the real local output step separately, and every direct call matches the updated signature. This conclusion is static; the new binary file-size test was inspected but not executed, and this does not establish restarted PW accumulation behavior.

std::stringstream file;
file << PARAM.globalv.global_stru_dir << "STRU_MD_" << mdrun->step_ + mdrun->step_rst_;
mdcell::print_stru_file(mdcell, mdcell.stru_meta(), file.str());
if (param_in.inp.out_stru > 0)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Honor the MD structure format selector

Updated out_stru help now supports MD and defines value 2 as CIF output, but this branch handles 1 and 2 identically and always writes STRU content without a .cif filename. Please implement the MD format choice or explicitly reject/document value 2 as unsupported for MD. Keep the restart structure requirement independent of the user-facing format.

}
}
MPI_Bcast(&header_ok, 1, MPI_INT, 0, comm);
MPI_Bcast(&base_offset, 1, MPI_OFFSET, 0, comm);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Preserve typed communication through the wrapper boundary

Non-blocking governance note: the pinned rule requires internally guarded parallel wrappers, while this diff adds an MPI_Bcast here and six direct rank/barrier calls in tests. Please use domain-aware wrappers where supported or record a narrow reviewed exception for the offset path. Preserve MPI_Offset width and the cell communicator; narrowing the offset to int or using a world-only broadcast would be unsafe.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: the new PW append test adds three direct MPI_Barrier calls and the NAO fallback test adds two more. Please use the internally guarded domain-aware barrier where supported, preserving the synchronization around file checks. Retain the earlier MPI_Offset-width and cell-communicator caveat for the separate production broadcast.

// container and zero-iteration invariants that keep empty partitions safe.
// ---------------------------------------------------------------------------

TEST_F(ElfLogicTest, ZeroGridHasLocalGrid)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Exercise the real empty-partition ELF path

Non-blocking: these zero-grid cases test local lambdas, vector construction and zero-iteration loops without calling write_elf. They would still pass if an empty-rank return were reintroduced before FFT/gather. Please add a production-writer or narrow instrumented MPI test that verifies collective participation with an empty partition; this can follow when runtime validation is available.

echo "Error: can not find file $ref or $cal"
return 1
fi
python3 $COMPARE_SCRIPT <(sed '/^[[:space:]]*#/d' "$ref") <(sed '/^[[:space:]]*#/d' "$cal") 4

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve the Vxc comparison tolerance

Non-blocking test-validity note: routing CompareOrbXC through this helper changes accuracy from 5 to 4, so CompareFile.py accepts absolute differences up to 1e-4 instead of 1e-5. The numeric writer still uses precision 16 and the renamed reference is unchanged. Please retain accuracy 5 for Vxc via a helper argument, or document and independently justify the intended tolerance change.

catch (const std::invalid_argument& e) {
ModuleBase::WARNING("Input", "out_mat_vnlk enable flag must be 0/1, using default 0");
}
if (count >= 2) try { para.input.out_mat_vnlk[1] = std::stoi(item.str_values[1]); }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Forward the selected matrix-component precision

The new component controls accept and document a second precision value, but write_hk_common always passes literal 8 to save_mat. This affects out_mat_vnlk, out_mat_vlk, out_mat_vhk, out_mat_vxck and out_mat_exxk, including legacy aliases; out_mat_tk already uses precision correctly. Please pass the selected component precision through the writer and verify a non-default value.

local wfc_ref_nohdr
wfc_cal_nohdr=$(mktemp)
wfc_ref_nohdr=$(mktemp)
sed '/^[[:space:]]*#/d' "$wfc_cal" >"$wfc_cal_nohdr"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Fail comparison when WFC preprocessing fails

In the documented plain-bash collector and Single_job/run_check paths, a missing gamma-only wf_nao.txt makes sed fail but leaves an empty temporary file, and that status is ignored. CompareFile.py loops only over the shorter input, so it records Compare_wfc_lcao_pass 0 against a nonempty reference. Please check source/preprocessing success before comparing; Autotest.sh is protected by bash -e, but these supported entry points are not.

abacus_fixer added 9 commits October 23, 2026 19:28
The preceding two commits wrapped the cal_gint_vl call in a pointer
guard, so a grid-parallel rank with nrxx == 0 skipped it entirely.
cal_gint_vl reaches an MPI_Alltoall in transferSerials2Parallels (via
hr_gint_to_hR); with rank 3 absent, ranks 0-2 blocked in the Alltoall
while rank 3 raced ahead into save_mat's Allreduce, deadlocking the
run. SCF never hangs here because every rank calls cal_gint_vl.

Re-enter cal_gint_vl on all ranks to keep the collective matched, and
only avoid the original matrix::operator() ic < nc assertion by passing
nullptr (never dereferenced, since such a rank owns no local biggrid)
instead of &v(ispin, 0) for the Hartree and XC terms. The local term
passes v_local directly (already a plain pointer, null on empty ranks)
and needed no change, so it is restored to the unconditional call.

Verified: examples/17_relax/04_relax_with_output_lcao with
out_mat_h_vh=1 under mpirun -np 4 now completes 5 relax steps in ~2s
(no assertion, no deadlock; previously it hung).
filename_output only omitted the extra 'k' k-point prefix for hk, sk and
tk. The potential terms written by hterm_writer use k-suffixed property
names (vhk, vlk, vnlk, vxck, vexxk) that already denote k-space
quantities, so they fell into the branch that prepends another 'k',
yielding doubled names like vhkk1s1_nao.txt.

The INPUT help for out_mat_h_vexx documents the intended pattern as
vexxk*_nao.txt (property directly followed by the k-point index), so
generalize the special case: any property whose name ends in 'k' takes
the bare index, everything else keeps the 'k' prefix. wf, vxc and other
non-k properties are unchanged.

Verified: examples/17_relax/04_relax_with_output_lcao with
out_mat_h_t/vnl/vl/vh/vxc enabled under mpirun -np 4 now writes
vhk1s1_nao.txt etc. (no doubled k), and ctest -R MODULE_BASE_filename
passes.
find_md_restart_stru only recognized the per-step "STRU{istep+1}" and
legacy "STRU_MD_{istep}" names, so a default MD restart could not find a
checkpoint written in out_app_flag == 1 mode, where run_md appends one
"# MD step N" frame per output step to a single fixed-name STRU file.

Add stru_frame_io::extract_last_frame() in source_cell to split the last
frame of a cumulative STRU file into a single-frame side file, and let
find_md_restart_stru fall back to that file so the restart readers see a
plain single-frame structure. The side file STRU_restart_frame is
rewritten on every restart to stay current with the latest frame.

Cover the frame extraction with a dedicated source_cell unit test
(MODULE_CELL_stru_frame_io) and extend the MD-restart routing case in
MODULE_IO_system_variable_test for the cumulative-STRU layout. Wire the
new source through the cell OBJECT library and Makefile.Objects.

Not yet compiled or run; build and test verification pending.
The mode-1 fixed-name cumulative file was appended to the read candidates
unconditionally. Since that file is always read from its first frame, a
positive restart index (istep >= 0) would silently load step-0 wave
functions whenever the step-suffixed file was missing, corrupting the
TDDFT restart without any error.

Restrict the cumulative-file candidate to istep < 0 (first read) so a
positive restart that cannot find its step-suffixed snapshot now fails
loudly through the existing error path instead of reading the wrong frame.

Add a unit test (ReadWfcNaoTest.PositiveStepRejectsCumulativeFile) that
writes only the cumulative file and asserts that a positive-step read
fails. Not yet compiled or run; build and test verification pending.
The new stru_frame_io unit test failed to link: the base library pulls in
base_device::memory::* symbols that live in the device target. Add device to
the test's link libraries, matching the sibling cell_geometry test.
P1-a made read_set_globalv.cpp call stru_frame_io::extract_last_frame, which
lives in the cell library. Test targets that compile read_set_globalv.cpp via
the io_input / io_input_serial OBJECT libraries but do not link cell then fail
to link with an undefined reference.

Add stru_frame_io.cpp to io_input_serial (covers all its consumers) and to the
three io_input-based tests that do not link cell. The main io_input library is
left untouched: the abacus binary already links cell, so adding the source
there would duplicate the symbol.
In append mode (out_app_flag == 1) the cumulative PDOS, DM(k) and MD-STRU
files used "istep >= 0" / unconditional append, so rerunning in an existing
OUT directory kept stale records from the previous run mixed with the new
data. Now the first output step of a fresh (non-restart) run truncates the
fixed-name file; later steps and restarted runs keep appending.

- write_pdos_text.cpp / write_dmk.cpp: append only when istep > 0
- run_md.cpp: do not append on the very first step of a fresh run
  (step_ == 0 && step_rst_ == 0)
- read_inp_out.cpp: document the mode-1 first-step-truncation contract in
  the out_app_flag help text
write_wfc_pw used a single istep both to decide whether the ionic step appears
in the file name and whether to append a new frame. When out_freq_ion == 0 the
caller passes istep = -1 (fixed filename), which made append_frame always
false, so every converged ionic step truncated the file instead of
accumulating one frame per step as documented.

Add an append_step parameter carrying the real output step: append_frame now
depends on append_step while istep remains only the filename sentinel. The
caller passes the real ionic step; single-point writes pass -1.

Add a WriteWfcAppendMode test that truncates on the first step, appends on the
second (file grows), and truncates again on a fresh run (size restored).
Verified: single-process and mpirun -np 4 both pass (2/2).
…_norm

The mode-2 (out_app_flag==2) subdirectory predicates in create_directory were
missing three output classes, so enabling them alone wrote files into
directories that were never created:

- out_hsr (formats 1 and 2, CSR text) writes to global_mat_dir but only
  out_hsr_npz was in the MAT/ predicate; add out_hsr[0]==1 || ==2.
- out_pchg writes to global_chg_dir but only out_chg was in the CHG/
  predicate; add !out_pchg.empty().
- out_wfc_norm / out_wfc_re_im write to global_psi_dir but only out_wfc_lcao /
  out_wfc_pw were in the PSI/ predicate; add !empty() for both.

Verified: io_input_serial compiles (make -j 30). The create_directory path
cannot be exercised in the serial unit test because make_dir_out calls
MPI_Barrier, so predicate coverage is deferred.

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static incremental review of 720f830 with independent A/B verification. Existing findings and fixes are updated in their original discussions; the new global-dependency finding is inline. git diff --check passed; no build, runtime or CI verification was performed. The inspected target merge has a CMake conflict, and GPU/fp32 empty-partition behavior remains unverified.

// second write never grew the file.
TEST_F(ReadWfcRhoTest, WriteWfcAppendMode)
{
const int kpar = GlobalV::KPAR;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Keep the PR global-dependency budget non-increasing

This new test adds 11 GlobalV references, changing the exact full PR budget to 53 added and 46 removed (net +7). The established governance includes tests in that budget. Please reuse explicit fixture/setup values and existing locals so the additional regression test does not increase global coupling, or document a reviewed exception; this is static accounting, not a claimed CI result.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Input&Output Suitable for coders without knowing too many DFT details Refactor Refactor ABACUS codes Solve before 3.11

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants