Skip to content

Refactor hsolver: move MatrixBlock to source_base, drop dead hamilt include - #7920

Merged
mohanchen merged 6 commits into
deepmodeling:developfrom
Critsium-xy:refactor/hsolver-neutral-matrixblock
Sep 9, 2026
Merged

Refactor hsolver: move MatrixBlock to source_base, drop dead hamilt include#7920
mohanchen merged 6 commits into
deepmodeling:developfrom
Critsium-xy:refactor/hsolver-neutral-matrixblock

Conversation

@Critsium-xy

Copy link
Copy Markdown
Collaborator

Reminder

  • I have read AGENTS.md and docs/developers_guide/agent_governance.md.
  • I have linked an issue or explained why this PR does not need one.
  • I have added adequate unit tests and/or case tests, or explained why not.
  • I have listed the exact verification commands run and their results.
  • I have described user-visible behavior changes, including INPUT parameter changes.
  • I have explained core-module impact for ESolver, HSolver, ElecState, Hamilt, Operator, Psi, or other source/ changes.
  • I have requested any needed governance exception below.

Linked Issue

No issue. This is the first, purely mechanical step of an incremental effort to
turn source_hsolver into a self-contained numerical module. It changes no
behavior and is small enough to review on its own, so it seemed better to send
it standalone than bundled with the interface changes that have to follow.

Unit Tests and/or Case Tests for my changes

Commands run (Linux, gcc, cmake 3.31 + ninja, 15 cores):

Full-feature build from scratch:

cmake -B build -G Ninja -DBUILD_TESTING=ON -DENABLE_LCAO=ON -DENABLE_MPI=ON -DENABLE_OPENMP=ON
cmake --build build -j15 -- -k 0

diago_cusolver.{h,cpp} are guarded by USE_CUDA and are the files this PR
changes most, so they were built explicitly:

cmake -B build-cuda -G Ninja -DBUILD_TESTING=OFF -DENABLE_LCAO=ON -DENABLE_MPI=ON -DENABLE_OPENMP=ON -DUSE_CUDA=ON
cmake --build build-cuda -j6 --target diag_cusolver

Whole unit-test suite, serially, on this branch and on the base commit
fb9ce1da4 built with the same flags:

OMP_NUM_THREADS=1 ctest --test-dir build -E '^(0[1-9]|1[0-9])_'

Governance check:

python tools/03_code_analysis/agent_governance_check.py --staged

Result summary

check result
full build 3301/3301 targets, 0 errors
diag_cusolver (CUDA) builds clean
unit tests, base fb9ce1da4 91% passed, 29 failed of 335
unit tests, this branch 91% passed, 29 failed of 335
governance check 0 blockers

The two failing sets are identical test-for-testdiff of the sorted
The following tests FAILED lists is empty. They are pre-existing failures of
the machine I have access to (mostly mpirun-launched *_para/*_parallel
wrappers, plus MODULE_BASE_math_sphbes, MODULE_BASE_cubic_spline,
MODULE_PSI_init_test, MODULE_HAMILT_vdwTest,
MODULE_RELAX_relax_new_relax) and are unrelated to this change.

Checks not run, with reason

  • -DENABLE_CUSOLVERMP=ON (diago_cusolvermp.cpp): cuSOLVERMp is not installed
    on that machine, so CMake refuses to configure (cuSOLVERMp not found. Set CUSOLVERMP_PATH or NVHPC_ROOT_DIR.). The change in that file is one
    hamilt::MatrixBlockModuleBase::MatrixBlock rename plus the matching
    include; after the alias both names denote the same type.
  • Integration cases under tests/: this PR changes only which header a
    declaration comes from and how a type is spelled — no code path, no
    arithmetic — so numerical results cannot move. CI's integration suites cover
    it.

What's changed?

source_hsolver is meant to be the numerical linear-algebra layer, but 16 of its
files reach into source_hamilt. Two of those reasons turn out not to be real:

  1. diago_bpcg.h included source_hamilt/hamilt.h and used nothing from
    it. DiagoBPCG::diag() has taken HPsiFunc/SPsiFunc closures for a while;
    the include is left over from before that.

  2. hamilt::MatrixBlock is a four-field aggregate that records where a matrix
    lives
    T* p, local row/col, and the BLACS desc. It carries no
    physics, but because it lived in source_hamilt, a solver that only wanted a
    matrix descriptor had to include the whole Hamiltonian interface to get it.

So this PR:

  • moves the struct to a new source/source_base/matrix_block.h as
    ModuleBase::MatrixBlock, field-for-field unchanged (still an aggregate,
    still no default member initializers, so the existing
    MatrixBlock<T>{p, row, col, desc} brace initializations keep working);
  • leaves using ModuleBase::MatrixBlock; in source_hamilt/matrixblock.h, so
    every hamilt::MatrixBlock spelling in source_estate, source_lcao,
    source_io, source_esolver and the hsolver tests compiles untouched. No
    file outside source_hsolver and those two headers is modified;
  • switches source_hsolver to the canonical ModuleBase::MatrixBlock, with
    source_base/matrix_block.h included directly.

Net effect:

before after
source_hsolver files including source_hamilt 16 13
#include "source_hamilt/..." lines there 17 13
hamilt::MatrixBlock in source_hsolver (non-test) 49 0
hamilt::MatrixBlock elsewhere in the tree unchanged unchanged

diago_bpcg.h, diago_cusolver.h and diago_scalapack.cpp no longer reference
source_hamilt at all. Each remaining include is hamilt.h pulled in for
hamilt::Hamilt (plus one module_xc/general_exx_info.h in
hsolver_lcaopw.cpp) — exactly the set a follow-up has to address. Because
those files now already spell the type canonically and include the base header
directly, dropping hamilt.h from them later becomes a pure signature change
with no name churn.

For context on the direction: DiagoCusolver::diag and
DiagoScalapack::diag_pool already take MatrixBlock rather than a
hamilt::Hamilt*, and DiagoCG/DiagoDavid/DiagoDavSubspace/DiagoBPCG
already take std::function closures instead of the Hamiltonian. The intent is
to make the rest of source_hsolver consistent with the parts that are already
decoupled, one reviewable step at a time.

Governance Notes

  • INPUT/docs changes: none. No Input_Item, no user-visible behavior, no
    output format is touched, so docs/parameters.yaml and
    docs/advanced/input_files/input-main.md are unaffected.
  • Core module impact: HSolver and Hamilt. After the alias,
    hamilt::MatrixBlock<T> and ModuleBase::MatrixBlock<T> name the same
    type, so every existing signature still matches — including
    hamilt::Hamilt<T>::matrix(MatrixBlock<T>&, MatrixBlock<T>&) and the mock
    overrides in source_hsolver/test, which are deliberately left as they are.
    There is no ABI or numerical change; object code differs only by the removed
    hamilt.h include in three translation units.
  • Exceptions requested: none. Two agent_governance_check.py warnings are
    expected and intentional:
    • "Header diff adds an include dependency" on 7 headers. Every added include
      is source_base/matrix_block.h, strictly narrower than the
      source_hamilt/hamilt.h it replaces or sits beside. Adding it explicitly
      even where hamilt.h still supplies it transitively is deliberate — it is
      what makes the later removal of hamilt.h safe.
    • "Source code changed without test path changes". This is an include and
      type-alias refactor with no behavior change, covered by the existing
      MODULE_HSOLVER_* suite, which is unchanged and shown above to have the
      same pass/fail set as the base commit. A new test for a type alias would not
      test anything.

🤖 Generated with Claude Code

Critsium-xy and others added 4 commits September 7, 2026 15:50
MatrixBlock only records a matrix memory layout -- data pointer, local
row/column counts and the BLACS descriptor -- so it carries no physics
and does not belong to the Hamiltonian module. Move the definition to
source_base/matrix_block.h as ModuleBase::MatrixBlock and leave a
`using` alias in source_hamilt/matrixblock.h, so the ~30 existing
hamilt::MatrixBlock spellings outside source_hsolver keep compiling
unchanged.

With that, three source_hsolver files stop reaching into source_hamilt
altogether:

- diago_bpcg.h included source_hamilt/hamilt.h without using a single
  symbol from it; DiagoBPCG::diag() has taken HPsiFunc/SPsiFunc
  closures for a while.
- diago_cusolver.h only needed the matrix descriptor, not the whole
  Hamiltonian interface. psi.h is now included explicitly, it used to
  arrive through hamilt.h.
- diago_scalapack.cpp likewise only needed the descriptor.

The source_hsolver files that still need hamilt::Hamilt now spell the
type ModuleBase::MatrixBlock and include source_base/matrix_block.h
directly, so dropping their source_hamilt/hamilt.h include later is a
pure signature change with no name churn.

source_hsolver -> source_hamilt includes: 16 files / 17 lines ->
13 files / 13 lines. No behavior change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up review fixes on top of the MatrixBlock move:

- parallel_k2d.h swapped one include for another, but the header itself
  never names MatrixBlock -- only parallel_k2d.cpp does. Drop it from the
  header and include source_base/matrix_block.h in the .cpp instead, so
  the header's dependency set actually shrinks rather than shifts.

- diago_cusolver.h includes source_basis/module_ao/parallel_orbitals.h,
  but Parallel_Orbitals appears in neither the header nor the .cpp. Same
  class of dead include as the source_hamilt/hamilt.h one already removed
  from diago_bpcg.h.

- source_hamilt/matrixblock.h is now a one-line alias kept only for the
  historical spelling. Say so with a TODO, so it has a stated end state
  instead of quietly becoming permanent.

No behavior change; no type or signature is touched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NxDLsUHq81NAbsh2rpCwKB
@mohanchen mohanchen added the Feature Discussed The features will be discussed first but will not be implemented soon label Sep 8, 2026
@mohanchen mohanchen added Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0 and removed Feature Discussed The features will be discussed first but will not be implemented soon labels Sep 9, 2026
@mohanchen
mohanchen merged commit d9d5206 into deepmodeling:develop Sep 9, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants