Repository navigation
Input: reject DSP runs where kpar*bndpar != NPROC at input time - #8238
Merged
mohanchen merged 3 commits intoOct 30, 2026
Merged
Conversation
On DSP hardware the mtblas/C_PACK vendor libraries require exactly one MPI process per k-point pool; other configurations make the vendor routines return inconsistent results across ranks and the SCF silently diverges. deepmodeling#6771/deepmodeling#7035 guarded this with a WARNING_QUIT in Driver::init_hardware, but deepmodeling#7420 removed the guard entirely to allow bndpar+kpar combinations. Reinstate the guard as a kpar input check under __DSP, requiring kpar (kpar_lcao for LCAO) * bndpar == NPROC, which keeps the bndpar+kpar use case from deepmodeling#7420 working. Also document the restriction in the kpar INPUT description. Fixes deepmodeling#8228
DanielZhao0432
force-pushed
the
fix-dsp-kpar-guard
branch
from
October 29, 2026 14:06
64dcb68 to
e3a21bd
Compare
Collaborator
Author
|
@ABACUS-PR-BOT Could you please take a look to see why two tests failed? If the issue is simple, help me fix it. |
ABACUS-PR-BOT
left a comment
Collaborator
There was a problem hiding this comment.
One DSP input-validation defect and one nonblocking test suggestion remain. This was a static source review; no builds or runtime tests were run, and the reported Tianhe results were not independently verified.
Collaborator
|
For the two CI failures, I couldn't retrieve the diagnostics. Please share the test names and the first failure message from each so I can assess the cause and whether a simple fix is possible. |
- Extract the DSP k-point/band layout arithmetic into the inline ModuleIO::dsp_kpar_layout_valid (read_input_tool.h) so it is covered by unit tests without a DSP build. - Skip the check when bndpar <= 0 (rejected by the bndpar input check), so the diagnostic message never divides by zero. - Add source/source_io/unittests/test_read_inp_sys.cpp covering PW k-point pools, the LCAO kpar->kpar_lcao reset, the bndpar exemption from deepmodeling#7420, and degenerate (zero/negative) values.
mohanchen
approved these changes
Oct 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Reminder
AGENTS.mdanddocs/developers_guide/agent_governance.md.source/changes.Linked Issue
Fix #8228
Unit Tests and/or Case Tests for my changes
(
read_inp_sys.cpp.o) successfully; the new check is inside#ifdef __DSPand is not compiled in non-DSP builds.
commit 64dcb68), case
001_4GaAs, 4 MPI processes x 4 OpenMP threads:kpar=1(default): ABACUS aborts during input reading with the intendeddiagnostic: "On DSP hardware, the mtblas/C_PACK libraries require kpar
(kpar_lcao for LCAO) * bndpar to equal the number of MPI processes, but got
NPROC=4, kpar=1, kpar_lcao=1, bndpar=1. Please set kpar=4 or adjust the
process count." (exit code 1)
kpar=4: the calculation runs and converges normally.python3 tools/03_code_analysis/agent_governance_check.py --staged→ no findings.source/source_io/unittests/test_read_inp_sys.cpp(targetMODULE_IO_test_read_inp_sys) coveringdsp_kpar_layout_valid: PW k-pointpools, the LCAO kpar→kpar_lcao reset, the bndpar exemption from Fix: relax the parameter constraints of device DSP to allow bndpar+kpar #7420, and
degenerate zero/negative values — 4/4 tests pass locally.
__DSPbuilds and CI has no DSP toolchain/hardware; the behavior was verifiedmanually on Tianhe as listed above.
What's changed?
one MPI process per k-point pool. Any other configuration (e.g. the default
kpar=1with 4 processes) makes the vendor routines return inconsistent resultsacross ranks, and the SCF silently diverges (see the linked issue for logs).
[Fix] Report and exit for unsupported DSP KPAR configuration #6771/Fix DSP count check problem #7035 previously rejected such configurations with a
WARNING_QUITinDriver::init_hardware, but Fix: relax the parameter constraints of device DSP to allow bndpar+kpar #7420 removed the guard entirely to allowbndpar+kpar combinations, leaving the invalid configuration unguarded.
kparinput check compiled only under__DSP:kpar(kpar_lcaofor LCAO) *bndparmust equal the number of MPIprocesses. Unlike the removed check, the
bndparfactor keeps the bndpar+kparuse case that Fix: relax the parameter constraints of device DSP to allow bndpar+kpar #7420 intended to allow. Because the check runs during input
reading, it also triggers under
--check-input.kparINPUT description now documents the DSP restriction (visible viaabacus -h kparand in the generated documentation).diagnostic for configurations the vendor libraries cannot handle, instead of
silently diverging mid-SCF. Non-DSP builds are completely unaffected.
Governance Notes
kpardescription updated (metadata only; no parameteradded/renamed; the new validation applies to DSP builds only).
source_io.Parameter::sys.nproc, populated byParameter::set_pal_paraminmain.cppbefore input reading). One expected warning: the header
read_input_tool.hgains an
<algorithm>include, required by the new inline helper'sstd::max.The low
code_quality_score.pyscores forread_inp_sys.cpp(0) andread_input_tool.h(48) are pre-existing historical debt (legacy macros, filelength); the new test file scores 95.