Skip to content

refactor: clean up source_cell/source_estate/source_io reverse dependencies#7521

Merged
mohanchen merged 6 commits into
deepmodeling:developfrom
Critsium-xy:refactor/cell-remove-dead-cereal-include
Jun 27, 2026
Merged

refactor: clean up source_cell/source_estate/source_io reverse dependencies#7521
mohanchen merged 6 commits into
deepmodeling:developfrom
Critsium-xy:refactor/cell-remove-dead-cereal-include

Conversation

@Critsium-xy

@Critsium-xy Critsium-xy commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Reminder

  • Have you linked an issue with this pull request?
  • Have you added adequate unit tests and/or case tests for your pull request?
  • Have you noticed possible changes of behavior below or in the linked issue?
  • Have you explained the changes of codes in core modules of ESolver, HSolver, ElecState, Hamilt, Operator or Psi? (ignore if not applicable)

Linked Issue

N/A — standalone refactor of the module dependency graph.

What's changed?

While auditing the cross-module #include graph under source/source_*, I found several cases where a lower-level module depended upward on a higher-level one, creating unnecessary reverse edges in the module dependency DAG. This PR removes three of them. Each change is an isolated, mechanical move with no behavioral / numerical change — only file locations, #include paths, and CMake source lists are touched. The relevant code is already covered by existing unit tests.

1. Remove a dead source_lcao include from source_cell
source_cell/read_atom_species.cpp carried a dead block:

#ifdef __EXX
#include "source_lcao/module_ri/serialization_cereal.h"
#endif

Nothing in the file uses it — it was a spurious cell → lcao reverse edge. Removed it and added the #include <sstream> the file actually relies on (std::stringstream, previously only available transitively).

bcast_cell.cpp still includes that header legitimately (it uses bcast_data_cereal, defined there), so that include is left untouched.

2. Move Magnetism from source_estatesource_cell
source/{source_estate => source_cell}/magnetism.{h,cpp} (100% rename). The Magnetism class only depends on source_base and is held by value as a member of UnitCell (unitcell.h). Living in source_estate forced a cell → estate reverse edge. Repointed all includers and updated the CMake/test source lists.

3. Move the output print helpers from source_io/module_outputsource_base
source/{source_io/module_output => source_base}/output.{h,cpp} (rename). class output is a pure formatter over ModuleBase types (realArray, matrix, matrix3, ComplexMatrix) with no dependency above source_base. Sitting in source_io forced several cell/pw/psi → io reverse edges. Repointed all includers, fixed the header's own internal includes, and updated the main + test CMake source lists.

Test build adjustments (consequence of moving output into base):
Because output.cpp is now part of the base OBJECT library that nearly every test links, two stale patterns had to be cleaned up to avoid duplicate-symbol errors:

  • Removed redundant explicit output.cpp entries from base-linked test SOURCES lists (kept where a test hand-compiles source_base/*.cpp and does not link base, e.g. source_md/test).
  • Removed now-conflicting no-op output::printM3 / printrm mock stubs from a few tests (symmetry_test_*, to_qo_test, read_atoms_helper_test, psi_initializer_unit_test); the real base implementation now provides those symbols. Each test's still-needed Magnetism / InfoNonlocal mocks were left intact.

Any changes of core modules? (ignore if not applicable)

No functional change to ElecState/Hamilt/Psi/etc. Magnetism (used by ElecState and UnitCell) and the output printer are physically relocated to lower-level modules, but their interfaces and behavior are unchanged; all call sites only have their #include paths updated.

Verification

  • Full build with -DBUILD_TESTING=ON (gcc/OpenMPI, LCAO enabled): 0 errors, all targets + unit-test executables link, including the full abacus_* binary.
  • Verified no residual references to the old source_estate/magnetism.h or source_io/module_output/output.{h,cpp} paths.
  • The legacy Makefile build is unaffected: Makefile.Objects uses VPATH, so object names resolve regardless of which source_* directory a file lives in.

🤖 Generated with Claude Code

Critsium-xy and others added 3 commits June 25, 2026 14:41
read_atom_species.cpp included source_lcao/module_ri/serialization_cereal.h
(guarded by __EXX) but uses no cereal/serialization symbols. Removing it
cuts a source_cell -> source_lcao reverse dependency edge.

Add the direct <sstream> include the file actually needs (std::stringstream),
which was previously only available transitively through the cereal header.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Magnetism class only depends on source_base and is held by value as a
member of UnitCell (source_cell/unitcell.h). It is a fundamental property of
the unit cell, so it belongs in source_cell rather than source_estate.

This removes the unitcell.h -> source_estate header dependency (a core data
structure no longer reaches up into the electronic-state module) and breaks
one direction of the source_cell <-> source_estate cycle.

- git mv source_estate/magnetism.{h,cpp} -> source_cell/
- update all includers to source_cell/magnetism.h
- move the source file entry between CMakeLists and fix the unit-test path

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `output` class (output.{h,cpp}) is a pure formatter that only operates on
source_base types (realArray, matrix, matrix3, ComplexMatrix). It was sitting
in source_io/module_output but has no dependency on anything above source_base,
so it belongs in source_base as a leaf utility.

Relocating it removes a batch of source_cell -> source_io (and source_pw/
source_psi -> source_io) reverse edges that existed only to reach this printer,
without changing any behavior or namespace.

- git mv source_io/module_output/output.{h,cpp} -> source_base/
- fix the relocated header's own includes (drop ../../source_base/ prefix)
- repoint all includers to source_base/output.h
- move the source entry from source_io to source_base CMakeLists and update
  the relative output.cpp paths in all affected unit-test CMakeLists

Verified: full abacus_basic_para executable builds and links cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Critsium-xy Critsium-xy force-pushed the refactor/cell-remove-dead-cereal-include branch from e8ac89c to 7e5acaa Compare June 26, 2026 06:24
@Critsium-xy Critsium-xy marked this pull request as ready for review June 26, 2026 08:01

@mohanchen mohanchen 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.

Great! LGTM.

@mohanchen mohanchen added Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0 labels Jun 27, 2026
@mohanchen mohanchen merged commit a02fe6a into deepmodeling:develop Jun 27, 2026
16 checks passed
Growl1234 added a commit to Growl1234/abacus-develop that referenced this pull request Jun 27, 2026
Growl1234 added a commit to Growl1234/abacus-develop that referenced this pull request Jun 27, 2026
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