Repository navigation
Conversation
The TB2J interface (issue deepmodeling#6608) reads the Fermi energy from ABACUS output to set the upper bound of the Green's-function integral, but the "EFERMI = ... eV" line was removed from output_efermi on 2025-06-22 and the CSR header of H(R) carried no Fermi information either. - output_log: output_efermi now takes elecstate::Efermi and prints " E_Fermi = <ef> eV" or " E_Fermi = <up> <down> eV # two fermi energies (up, down)" when two_efermi is set. - hsr_writer: write_hcontainer_csr accepts an extra efermi_eV argument and appends ", E_Fermi = <value> eV" to the "# spin index" header line only when label == "H". write_hsr threads elecstate::Efermi through and converts each spin channel via Efermi::get_efval(ispin). - esolver_fp / ctrl_scf_lcao: pass pelec->eferm down to the writers. - write_dh / hterm_writer: pass 0.0 for the Fermi argument since dH/dR and other derived terms are not Hamiltonians and must not carry the Fermi annotation. - unit tests: migrate OutputEfermiTest to the Efermi signature, add TestSingleFermi / TestTwoFermi content checks; extend hsr_writer unittests with per-channel Fermi header coverage and an "S(R) has no E_Fermi" assertion; link fp_energy.cpp into MODULE_IO_hsr_writer_test. - integration reference: refresh scf_out_hsr/hrs1_nao.csr.ref with the new header line; remaining hrs*.csr.ref files will follow after their reference Fermi values are confirmed.
added 5 commits
October 2, 2026 21:44
- test_hsr_writer: expect setprecision(6) significant-digit output (5.4321 / 3.2109) instead of fixed 6-decimal strings; write the two spin channels to separate files since istep=0 opens in truncate mode and the second write was overwriting the first. - outputlog_test: recompute expected E_Fermi values with the actual ModuleBase::Ry_to_eV = 13.605698 constant instead of 13.605693122994. Verified: ctest -R "MODULE_IO_hsr_writer_test|MODULE_IO_output_log_test" passes 3/3.
Report each matrix file with a consistent "Write <X> matrix in NAO basis to file:" line in running log so users can see the write action: H(R), S(R), T(R), r(R), and dH/dR components now all follow the eig/occ message style. Drop the redundant "#Print out dH/dR components#" banner. Pass ofs_running into write_hsr, out_lat_r, and out_rR instead of reading GlobalV inside the IO layer.
Print "Write H(R) (<term> term) matrix in NAO basis to file: <fname>" for the individual Hamiltonian terms (kinetic/nonlocal/local/hartree/ xc/exx) when out_mat_h_t/vnl/vl/vh/vxc/exx writes their H(R) CSR files. Thread ofs_running through WriteHParams and gather_and_write instead of reading GlobalV inside the IO layer.
Print "Write dH/dR (<term> term) matrix in NAO basis to file: <fname>" for each dH/dR component CSR file written by out_mat_dh and out_mat_dh_t/vnl/vl/vh/vxc/exx (including the Pulay variants and the total dH sum). Thread ofs_running through WriteDHParams and pass a term name into write_dh_perI instead of reading GlobalV in the IO layer.
Add "Write <X> matrix in NAO basis to file" running-log lines for the matrix outputs that previously stayed silent: DM(R) (out_dmr), DM(k) (out_dmk, replacing a commented-out cout), dS/dR (out_mat_ds, and fix the label that hardcoded "dH/dR" for dS files), T(k) (out_mat_tk), L(R) (out_mat_l, replacing the large banner), and Vxc(R) (out_mat_xc2). Mark spin-polarized channels as "(spin up )"/"(spin down)" padded to equal width before the word "matrix" so consecutive lines stay aligned. Thread ofs_running through the IO functions (write_dmr, write_dmk, write_Vxc_R) instead of reading GlobalV inside the IO layer.
…sages Move the generic "Write data to file" message out of the low-level ModuleIO::write_cube (shared by all volumetric outputs) up into write_vdata_palgrid, which knows the spin component. Add a data_desc parameter (default "data") so the charge/potential call sites can pass "charge density" and "potential", and annotate spin-polarized files with "(spin up )"/"(spin down)". Do the same for out_band: thread nspin0 into nscf_bands so bands1/bands2 are labeled by spin channel. eig_occ.txt holds all spins in one file and stays unlabeled.
Cache the global running-log stream in a local std::ofstream& at the top of ctrl_scf_lcao and pass that reference onward, instead of naming GlobalV::ofs_running at each call site. This drops the repeated global references (13 -> 1) and brings the PR's global-dependency budget back below zero.
# Conflicts: # source/source_io/module_dhs/write_dh.h # source/source_io/module_dhs/write_dh_terms.cpp
Critsium-xy
reviewed
Oct 6, 2026
added 2 commits
October 6, 2026 16:50
Add the ', E_Fermi = 10.3798 eV' annotation to the '# spin index' line of hrs1_nao.csr.ref and hrs2_nao.csr.ref so the reference matches the output of write_hcontainer_csr. The Fermi value (10.3798 eV) is taken from the last E_Fermi row of the energy table in running_scf.log.
… write_hcontainer_csr
The previous implementation keyed the Fermi-energy annotation on the
string label == "H" and used 0.0 as a sentinel for 'no Fermi value'.
That silently emitted 'E_Fermi = 0 eV' when a caller wrote an H(R)
file without a Fermi value available (e.g. the gamma-folded test),
which looks like a real value.
Add an explicit 'const bool has_efermi' parameter to write_hcontainer_csr.
The spin-index header line now appends ', E_Fermi = <value> eV' iff
has_efermi is true, independent of label. Update all call sites:
- write_hsr: H(R) -> true, S(R) -> false
- write_h_term (T/V^NL/V^L/V^H/V^XC): false
- write_dh_perI (dH/dR components): false
- unittests: pass true/false explicitly
Add a Not(HasSubstr("E_Fermi")) assertion to GammaFoldedHeaderKeepsCsrReadable
to lock in that label="H" + has_efermi=false produces no Fermi line.
added 7 commits
October 6, 2026 17:10
output_efermi took 'std::ofstream& ofs_running = GlobalV::ofs_running' as a default argument, which hides the GlobalV dependency at every call site that omits it. Remove the default so the stream is always passed explicitly; update the single production caller in esolver_fp.
…output_log.h output_convergence_after_scf, output_after_relax, and output_vacuum_level each declared 'std::ofstream& ofs_running = GlobalV::ofs_running'. Remove all three defaults so the stream is always passed explicitly at every call site, matching the treatment of output_efermi in the previous commit. Update the two production callers that relied on the default: - esolver_fp.cpp: pass GlobalV::ofs_running to output_convergence_after_scf - write_elecstat_pot.cpp: pass GlobalV::ofs_running to output_vacuum_level With this change, no function in output_log.h carries a GlobalV default argument. The GlobalV references added at the two call sites are explicit rather than hidden behind a default.
The comment claimed 'dH/dR and similar derived terms are not the Hamiltonian', but gather_and_write writes the H(R) component term matrices (T, V^NL, V^L, V^H, V^XC, V^EXX), not dH/dR. dH/dR output lives in write_dh.cpp. Reword the comment to reflect that these are parts of the Hamiltonian, not the full H(R), and therefore do not carry the Fermi energy annotation.
write_dmk emitted one 'Write DM(k) ... matrix to file: <fn>' line per (k-point, spin) file. With dense k-meshes this noticeably bloats running_*.log. Remove the per-file log line from the inner loop and emit one summary line after the loops finish (rank 0 only), giving the output directory and the total number of files written. Failed file creation still triggers a WARNING for each failed file.
…line" This reverts commit 3df36f9.
The previous commit in this PR replaced the 47-line README (root-cause analysis of the np>=3 stress chaos, measured deviations per np/OMP setting, threshold rationale, and follow-up fix plan) with a one-line summary. That change is unrelated to this PR (Fermi energy output and running-log message unification) and dropped valuable diagnostic information. Restore the original README in full; the threshold change should be proposed in a separate PR that also keeps the rationale and the planned fixes.
The only existing write_hsr test uses out_type=2 (binary), which skips the Fermi-energy branch entirely. Add WriteHsrTextCsrCarriesPerSpinFermiFromEfermi which drives write_hsr with out_type=1, two_efermi=true, ef_up=0.5 Ry and ef_dw=0.3 Ry, and asserts that hrs1_nao.csr / hrs2_nao.csr carry ef_up*Ry_to_eV / ef_dw*Ry_to_eV in the '# spin index' header line. This covers the actual fix path in write_hsr: eferm.get_efval(ispin) * ModuleBase::Ry_to_eV which was previously untested (the write_hcontainer_csr tests pass eV values directly, bypassing the conversion).
Critsium-xy
reviewed
Oct 6, 2026
added 6 commits
October 8, 2026 09:20
The WriteHsrTextCsrCarriesPerSpinFermiFromEfermi case crashed with an unknown C++ exception / segfault when the unit-test build defines __MPI but runs on one rank: - write_hsr builds a serial pv and calls gatherParallels; passing iat2iwt=nullptr with nat=0 made get_nrow_atom(0) throw on the gather path. - gatherParallels also reaches get_indexes_row(iat) on the source pv, which dereferences iat2iwt_; init_serial_orbitals never sets it. - append=false with istep=0 produced hrs*g1_nao.csr names while the test read hrs*_nao.csr. Install a real iat2it/iat2iwt map, set the atomic trace on the source pv, and switch the call to append=true so the generated names match the asserted files. Remove the generated S(R) file afterwards. Verified: make -j 30; MODULE_IO_hsr_writer_test 21 passed + 1 expected skip, MODULE_IO_hsr_writer_test_parallel (np=2) passed (2/2 ctest).
Pass the running-log stream explicitly from the esolver/control boundary down to the leaf output writers instead of reading GlobalV::ofs_running inside the IO layer. Leaf writers gain a std::ofstream& ofs_running parameter: - write_vdata_palgrid, save_dH_sparse, write_elf, write_chg_init, write_pot_init Mid-layer functions updated (signatures, definitions, explicit instantiations, and internal call sites): - write_bands / nscf_bands - output_dHR / output_dSR / output_SR / output_TR - output_mat_sparse - Cal_ldos::cal_ldos_lcao, cal_ldos_pw / stm_mode_pw / ldos_mode_pw - Get_pchg_pw / Get_wf_pw (begin / write_separate / write_summed / write_norm / write_complex / write_cube) - ctrl_iter_pw / ctrl_scf_pw / ctrl_runner_pw - ctrl_runner_lcao, ctrl_output_fp - write_dos_pw esolver boundary passes GlobalV::ofs_running: esolver_ks, esolver_ks_pw, esolver_ks_lcao, esolver_fp, esolver_gets. Non-IO boundary call sites (esolver_dm2rho, write_elecstat_pot, exciton_plotter, get_pchg_lcao, get_wf_lcao) pass GlobalV::ofs_running directly. Tests updated to pass a std::ofstream instance: rho_io_test, write_bands_test, test_dhs_sparse_writer. No INPUT behavior change; docs not required.
- cube_io.h: remove `data_desc = "data"` default; pass explicit descriptive strings at all 22 call sites (charge density, partial charge, effective/electrostatic potential, kinetic energy density, electron localization function, local DOS, wave function norm/real/imag, exciton density). - write_bands.h: remove `nspin0 = 1` default; pass nspin0=1 explicitly at 4 test call sites in write_bands_test.cpp. - Normalize cube filenames to lowercase with consistent short prefixes: SPIN1_CHG.cube -> chgs1.cube (esolver_dm2rho) LDOS_<en>eV.cube -> ldos_<en>ev.cube (cal_ldos + stm.py + catch_properties.sh) Exciton_avg_<type>_state<n>_spin<s>.cube -> exc_<type>_st<n+1>_s<s+1>.cube (exciton_plotter; indices now start from 1) - Rename test reference: LDOS.cube.ref -> ldos.cube.ref (git mv). - Update result.ref and README to match new filenames. Verification: make -j 30 passed; ctest 7/7 passed (rho_io, write_bands, write_bands_parallel, hsr_writer_test, hsr_writer_test_parallel, dhs_sparse_writer_test, output_mat_sparse_test); code_quality_score average 74.9 (pass line 60), 3 files below 60 are pre-existing debt (tab_indentation, global_dependency, too_many_parameters in cal_ldos/ctrl_output_fp/exciton_plotter).
- chg_init.cpp: read_kin_file now probes and reads tau.cube (nspin==1)
/ taus1.cube (nspin>1) instead of the legacy SPIN1_TAU.cube /
SPIN<n>_TAU.cube. Aligns with the charge read path (chg.cube /
chgs1.cube) in the same file.
- test_chg_extra.cpp: remove dead `std::remove("./support/OLD2_SPIN1_CHG.cube")`
in ExtrapolateChargeCase4; no code path generates that file.
Verification: make -j 30 passed; ctest 8/8 passed (MODULE_CHARGE_extra +
the 7 IO tests from the prior commit); code_quality_score chg_init.cpp
64 (pass), test_chg_extra.cpp 57 (pre-existing debt: tab_indentation,
uppercase_constant, global_dependency).
Critsium-xy
reviewed
Oct 9, 2026
Critsium-xy
left a comment
Collaborator
There was a problem hiding this comment.
Follow-up review on fa75532.
Verification: ran agent_governance_check.py --base 4c69130 --head fa75532 (0 errors, 46 warnings, GlobalV net delta -5) and code_quality_score.py on the 66 changed C++ files; static cross-check of every write_vdata_palgrid call site, every hrs*.csr.ref, and renamed-filename references across tests/, tools/ and docs/. Not compiled and no tests executed locally, so the make -j 30 / ctest results in the body are unverified here.
Critsium-xy
reviewed
Oct 9, 2026
added 5 commits
October 9, 2026 20:02
Address remaining review comments on PR deepmodeling#8068: - Let callers own the spin tag via make_data_desc() in cube_io.h; the removed write_cube.cpp inference mislabeled is=-1/1-based/summed calls as "(spin down)" or "(spin up)". nspin=4 magnetic channels now log magnetization density mx/my/mz (chgs), effective magnetic field bx/by/bz (pots, holding B_xc) instead of bare "charge density" / "effective potential" for all four channels. - Drop the 5 pre-write cube log lines duplicated by write_vdata_palgrid. - Sync hs_matrix.md CSR header example with the E_Fermi annotation. - Rename the conditional-slice .dat to the 1-based exc_cond_ style so one run no longer mixes Exciton_cond_..._state<N> (0-based) with exc_..._st<N+1> names for the same state. - Strip the 6-digit E_Fermi header annotation before the 1e-8 CSR comparison in catch_properties.sh to avoid platform-dependent rounding failures; matrix tokens still compare at 1e-8.
# Conflicts: # source/source_io/module_ctrl/ctrl_output_fp.cpp
ReadFile() pre-checked inputs with os.path.isfile(), which returns False for the /dev/fd/<n> pipes produced by bash process substitution <(...), so the CSR comparisons in catch_properties.sh (added in 23d0ee4 to strip the E_Fermi header annotation) aborted with "can not find file". Open the path directly and catch OSError instead; missing files still report the same error and exit code.
# Conflicts: # tests/integrate/tools/catch_properties.sh
This branch has not been deployed
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
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.
fix part of #6608 fix #8096
Summary
This PR improves the readability and consistency of ABACUS IO output (running log and
.cubefiles) and removes a class of default-argument debt (Rule 5) from the IO layer. The work is split into three themes that build on each other.1. Running-log messages: label what is being written
The running log previously contained many bare
Write data to file: ...lines that did not say what data was written. This PR adds descriptive labels and spin tags so each line is self-describing:out_mat_h,out_mat_dh,out_mat_*) now reportWrite H matrix ...,Write dH matrix ..., etc., with per-spin/channel tags where applicable.data_descstring at every call site (charge density, partial charge, effective/electrostatic potential, kinetic energy density, electron localization function, local DOS, wave function norm/real/imag, exciton density). The spin tag is owned by the caller throughmake_data_desc(cube_io.h) instead of being inferred fromisinsidewrite_vdata_palgrid, so spin-summed (is = -1) and 1-based-index callers are labeled correctly. nspin=4 magnetic channels are labeledmagnetization density m{x,y,z}(rho[1..3]) andeffective magnetic field b{x,y,z}(the B_xc components of the potential) instead of bare "charge density" / "effective potential" for all four channels.Writing cube file <name>log lines inget_pchg_lcao.cpp/get_wf_lcao.cppthat duplicated theWrite ... to file:line now emitted bywrite_vdata_palgrid.has_efermiflag instead of string-matching the label, and the running-log banner for Fermi energy is emitted through a single path. The CSR header example indocs/advanced/elec_properties/hs_matrix.mdis updated for the, E_Fermi = <value> eVannotation.Tests:
test_hsr_writer.cppgains a text-CSR case covering per-spin Fermi sourced fromEfermi; fixed under single-rank__MPI(theParallel_Orbitalssource must callset_atomic_tracebeforegatherParallels, otherwiseget_nnrow_atomthrows onnat==0).hrs*.csr.refforscf_out_hsr_spin2to carry theE_Fermiheader.hsr_writerandoutput_logunittests.2. Cube filename normalization
.cubefilenames are now lowercase, short, and consistent with thechg.cube/chgs1.cube/pot.cube/wfi1s1k1re.cubefamily. Indices start from 1 and carry a short suffix indicating their meaning (s= spin,k= k-point,g= geometry step,st= state,i= band index).SPIN1_CHG.cube→chgs1.cube(esolver_dm2rho).LDOS_<en>eV.cube→ldos_<en>ev.cube(cal_ldos; updatedstm.py,catch_properties.sh,result.ref,README;git mvLDOS.cube.ref→ldos.cube.ref).Exciton_avg_<type>_state<n>_spin<s>.cube→exc_<type>_st<n+1>_s<s+1>.cube(exciton_plotter; indices now start from 1). The matching.datslice filename is renamed the same way, and the conditional-slice.datbecomesexc_cond_<type>_slice_st<n+1>.datso one run no longer mixesExciton_cond_..._state<N>(0-based) withexc_..._st<N+1>names for the same state.chg_init.cppreads tau fromtau.cube/taus1.cube(instead of legacySPIN1_TAU.cube/SPIN<n>_TAU.cube), aligning with the charge read path. The probe distinguishesnspin==1vsnspin>1.std::remove("./support/OLD2_SPIN1_CHG.cube")intest_chg_extra.cpp(no code path generates that file).SPIN1_CHG.cubeetc.) are intentionally left in place.3. Parameterize ofs_running through IO output chains (Rule 5)
The IO output layer used to read
GlobalV::ofs_runningdirectly in many leaf and mid-level functions. This PR threadsstd::ofstream& ofs_runningexplicitly through:write_vdata_palgrid(cube_io.h),write_bands/nscf_bands,save_dH_sparse/output_dHR/output_dSR/output_TR/output_SR/output_mat_sparse,cal_ldos_lcao/cal_ldos_pw/stm_mode_pw/ldos_mode_pw,write_elf,write_chg_init/write_pot_init,Get_pchg_pw/Get_wf_pwbegin + write_cube helpers,Get_pchg_lcao/Get_wf_lcaobegin,ctrl_iter_pw/ctrl_scf_pw/ctrl_runner_pw/ctrl_runner_lcao/ctrl_output_fp, plus their explicit template instantiations (CPU/GPU, double/complex).GlobalV::ofs_runningonce at the boundary; the IO layer itself no longer reads the global.Additionally, two historical default arguments are removed (Rule 5) and every caller now passes the value explicitly:
cube_io.h: removeddata_desc = "data"; all 22 call sites pass a descriptive string (see theme 1).write_bands.h: removednspin0 = 1; 4 test call sites inwrite_bands_test.cppnow pass1explicitly.Governance notes
GlobalV::ofs_running; reads are now concentrated at the esolver/control boundary (onestd::ofstream& ofs_running = GlobalV::ofs_running;alias inctrl_scf_lcao.cppis a local alias, not a new global read).write_cube'sndata_line = 6remains as pre-existing debt (out of scope here).<fstream>additions in headers that now carrystd::ofstream¶meters; each is required by the new signature.Test plan
make -j 30passes.ctest --test-dir build -V -R "rho_io|write_bands|hsr_writer|dhs_sparse|output_mat_sparse|CHARGE_extra"— 8/8 pass (MODULE_CHARGE_extra,MODULE_IO_rho_io,MODULE_IO_write_bands,MODULE_IO_write_bands_parallel,MODULE_IO_hsr_writer_test,MODULE_IO_hsr_writer_test_parallel,MODULE_IO_dhs_sparse_writer_test,MODULE_IO_output_mat_sparse_test).python3 tools/03_code_analysis/code_quality_score.pyon the 66 changed C++ files at head2427fe3afe: average 73.5 (pass line 60), 52/66 pass. 14 files below 60 are pre-existing debt (tab_indentation, global_dependency, too_many_parameters) not introduced by this PR:ctrl_scf_lcao.cpp0,esolver_ks_lcao.cpp31,ctrl_output_pw.cpp32,cal_ldos.cpp33,dhs_sparse_writer.cpp41,ctrl_output_fp.cpp43,ctrl_runner_lcao.cpp50,esolver_ks_pw.cpp55,cal_ldos.h55,exciton_plotter.cpp56,test_chg_extra.cpp57,esolver_fp.cpp58,esolver_gets.cpp58,pos_op_writer.cpp59.CASES_CPU.txt/CASES_GPU.txt(03_NAO_multik): newly enablescf_out_hsr_spin2— it was previously in neither CI list, and itshrs{1,2}_nao.csr.refwere refreshed in this PR for theE_Fermiheader, so CI now covers the new format. Three commented-out placeholders (#scf_out_hsr_npz,#scf_out_hr_npz,#scf_out_dm_npz) are reserved for future npz cases and have no effect.catch_properties.shstrips the, E_Fermi = ... eVheader annotation (printed withsetprecision(6), ~1e-4 eV resolution) before the 1e-8 CSR token comparison, avoiding platform-dependent rounding failures on the header value; matrix tokens still compare at 1e-8.tests/integrate) — to be run on the CI side;catch_properties.shupdated for the newldos_<en>ev.cubefilename.workflow_dispatch-only workflows (e.g.interface.yml).