Skip to content

Add native periodic SCC-DFTB2/DFTB3 support - #8035

Open
liu-687 wants to merge 26 commits into
deepmodeling:developfrom
liu-687:feature/native-dftb-beta9
Open

liu-687 wants to merge 26 commits into
deepmodeling:developfrom
liu-687:feature/native-dftb-beta9

Conversation

@liu-687

@liu-687 liu-687 commented Sep 26, 2026 •

Copy link
Copy Markdown

Linked Issue

No linked issue. This PR proposes an experimental native DFTB backend and does not close an existing issue.

What's changed?

  • Adds an experimental, in-process periodic SCC-DFTB solver, selected explicitly with basis_type=dftb and esolver_type=dftbnative. The default Kohn–Sham path is unchanged.
  • Implements SCC-DFTB2 and the interatomic third-order charge correction corresponding to DFTB+'s ThirdOrderFull, within the supported undamped-gamma model. The implementation includes a legacy SKF s+p basis, generalized Hermitian eigensolving, Mulliken SCC charges, linear/Pulay/modified-Broyden mixing, finite-temperature occupations, SKF spline repulsion, 3D Ewald electrostatics, and an optional frozen-SCC band path.
  • Writes a detailed OUT.{suffix}/dftb.log with setup, parameter, SCC-iteration, energy, and Mulliken-population data, alongside eigenvalue, occupation, charge, and optional band files. The DFTB log is flushed at each SCC iteration. This solver requires SKF parameter files, but does not require UPF pseudopotentials, ABACUS .orb files, or a DFTB+ runtime.
  • Rejects basis_type=dftb unless esolver_type=dftbnative during INPUT validation, with a direct diagnostic. The default solver and basis remain ksdft and pw.

Motivation and design

This backend is for workflows that need ABACUS INPUT/STRU/KPT handling and output conventions without packaging a DFTB+ executable or shared library. DFTB+ also offers an in-process API; using it from ABACUS would still require the external library and an adapter. The native backend is not API-compatible with DFTB+ and does not target feature parity. This trades a smaller runtime dependency footprint for additional maintenance and validation work.

The solver is included in standard builds and is opt-in at runtime through esolver_type=dftbnative; no ENABLE_DFTB_NATIVE build switch is added. This keeps the build configuration uniform while the solver's experimental status and supported scope are stated in the documentation and at runtime.

Inputs and supported scope

Use a standard ABACUS INPUT, STRU, and KPT, select basis_type=dftb and esolver_type=dftbnative, and provide a DFTB-native control file (default: dftb_native.in) pointing to the SKF parameter set. DFTB-specific controls include SCC convergence, mixing, temperature, third-order correction, and an optional band-path file. The default DFTB3 correction is off; enabling it requires a Hubbard derivative for each species.

The current model supports periodic 3D cells, legacy SKF files with a repulsive Spline section, an s+p basis, neutral valence electron counts, spin-degenerate calculations (nspin=1), monopole SCC charges, undamped gamma corrections, dense diagonalization, and frozen-potential band paths. Hydrogen-labelled species are rejected because the DFTB+ HCorrection = Damping model and its third-order derivatives are not implemented. Spin polarization, shell-resolved charges, modern SKF formats, dispersion, DOS/PDOS, restart, forces/stress, structural relaxation, and molecular dynamics are outside the current scope. Slab calculations use 3D-periodic Ewald electrostatics and require vacuum-convergence checks.

Unit Tests and/or Case Tests for my changes

  • Adds three CTest variants for the 36-atom C2N-h2D DFTB3 reference: the standard one-rank run, a two-rank MPI run, and a separate one-rank run with the C/N species blocks reversed in STRU. Each compares all 17,424 frozen-SCC band eigenvalues (121 k-points × 144 bands) against the DFTB+ 25.1 reference rounded to 1 meV; none requires a DFTB+ executable.
  • The checked-in comparison records a maximum absolute difference of 5.12700e-4 eV, a mean absolute difference of 2.49089e-4 eV, and an RMS difference of 2.87890e-4 eV. The regression limits are 7.5e-4 eV maximum and 3.5e-4 eV RMS.
  • Passed on commit cb77165 in CI: ctest --test-dir build -V --timeout 1700 -R '^(integrated_test|dftb_native_c2n_reference|dftb_native_c2n_mpi_2rank|dftb_native_c2n_reversed_species_order)$'; the integration/native DFTB regression step passed with all three C2N variants.
  • Passed on commit cb77165 in CI: ctest --test-dir build --output-on-failure --timeout 300 -R '^MODULE_DFTB_native$' (DFTB unit target passed, including the periodic self-image half-list regression).
  • Passed on commit cb77165 in CI: ctest --test-dir build -V --timeout 1700 -R MODULE_IO; the MODULE_IO step passed, including the updated general-help test.
  • Unit tests cover the generalized eigensolver, finite-temperature occupation endpoints and zero-weight k-points, charge mixing, periodic SCC, frozen-SCC bands, energy consistency, non-finite input handling, periodic self-image repulsion under the canonical half-list convention, incompatible DFTB basis/solver rejection, and the default Kohn–Sham selection.
  • Updated the general CLI --help text to list basis_type=dftb. The generated INPUT parameter reference already documents this value, so no generated parameter documentation needed regeneration for the help-text correction.

Contributor checklist

  • Reviewed AGENTS.md and the repository governance guide.
  • No matching existing issue was found; this PR adds an experimental solver and does not close an existing issue.
  • Added unit tests and a reproducible numerical regression.
  • Listed verification commands and results.
  • Described user-visible behavior and the effects on solver selection, parameter parsing, and cell/species setup.
  • No governance exception is requested.

Comment thread CMakeLists.txt Outdated
@Growl1234

Growl1234 commented Sep 26, 2026 •

Copy link
Copy Markdown

Frankly speaking, I dislike the duplication. If there's already dftb+ library as a code path that performs well, why do we need then an internal code path which is possibly not in sync with the other one and thus causing an additional maintenance burden. Moreover this feature isn't expected to be widely used. I vote for an external dftb+ dependency.

@Growl1234

Growl1234 commented Sep 27, 2026 •

Copy link
Copy Markdown

Thanks for explicitly documenting the tradeoff. I'm okay with this design.

Regarding CMake option, however, I would keep my opinion against adding it. Actually, the new explanation reinforces my thought: if there is no dependency reason (e.g. enabling DFT+D4 pulls in dftd4 library as dependency) or significant compile-time or binary-size reason (e.g. enabling an optional and non-essential feature pulls in a large number of objects to build with) for disabling it, then this option is not really expressing a build configuration; it is being used to encode a release/support policy in CMake, which definitely shouldn't be the case.

Once an internal implementation is merged into ABACUS, it should be compiled as part of normal builds. esolver_type=dftbnative is already an explicit runtime opt-in, and its experimental status can be clearly documented and warned about at runtime. Keeping it OFF by default also creates one more unnecessarily different ABACUS binaries depending on how they were configured. If the implementation is not yet ready to be present in a standard build, I would rather regard that as a merge-readiness issue than introduce another build-time feature switch.

P.S. In fact, I had previously wanted to make the same argument against ENABLE_LCAO, but that option has existed for years and I wasn't sure whether the maintainers would be willing to change such long-standing behavior.

[Update: Thank you for considering my proposal.]

@liu-687
liu-687 marked this pull request as ready for review September 27, 2026 15:54
Copilot AI lite review requested due to automatic review settings September 27, 2026 15:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Critical build, SKF-data, and periodic-repulsion issues remain unresolved, along with input and test integration issues.

Review effort: Lite
Findings: 4 High severity · 2 Medium severity

Open (6)
What changed in this PR

Adds an experimental native periodic SCC-DFTB2/DFTB3 solver with SKF parsing, SCC mixing, band outputs, documentation, tests, and CI coverage.

Changes:

  • Adds native DFTB solver selection and input handling.
  • Implements periodic Hamiltonian construction, eigensolving, mixing, electrostatics, and output.
  • Adds C2N regression data, unit tests, documentation, and build integration.
File Summary
tests/​integrate/​CMakeLists.txt Registers the DFTB regression test.
tests/​03_DFTB_native_C2N/​STRU Defines the C2N test structure.
tests/​03_DFTB_native_C2N/​run_regression.py Runs and validates the band regression.
tests/​03_DFTB_native_C2N/​reference/​README.md Documents reference data.
tests/​03_DFTB_native_C2N/​reference/​comparison_summary.txt Records comparison metrics.
tests/​03_DFTB_native_C2N/​README.md Documents the regression case.
tests/​03_DFTB_native_C2N/​parameters/​README Documents bundled SKF parameters.
tests/​03_DFTB_native_C2N/​parameters/​LICENSE Includes parameter licensing terms.
tests/​03_DFTB_native_C2N/​KPT Defines the integration mesh.
tests/​03_DFTB_native_C2N/​INPUT Selects the native DFTB solver.
tests/​03_DFTB_native_C2N/​dftb_native.in Provides DFTB settings.
tests/​03_DFTB_native_C2N/​dftb_band_path.in Defines the band path.
source/​source_io/​module_parameter/​read_inp_sys.cpp Adds DFTB inputs and validation. Moderate (2 votes): basis_type=dftb is not rejected with incompatible solvers.
source/​source_io/​module_parameter/​read_inp_estruc.cpp Adds the DFTB basis option. Moderate (1 vote): inverse solver/basis validation is missing. Nit (1 vote): general help omits dftb.
source/​source_io/​module_parameter/​input_parameter.h Stores native DFTB configuration.
source/​source_esolver/​esolver_factory.cpp Registers native solver construction.
source/​source_esolver/​esolver_dftb_native.h Declares the solver interface.
source/​source_esolver/​esolver_dftb_native.cpp Implements solver lifecycle. Critical (1 vote): bundled N–N SKF metadata yields invalid valence and Hubbard values.
source/​source_esolver/​esolver_dftb_native_output.cpp Writes DFTB results and band outputs.
source/​source_esolver/​esolver_dftb_native_input.h Declares native input parsing.
source/​source_esolver/​esolver_dftb_native_input.cpp Parses DFTB settings, k-points, and band paths.
source/​source_esolver/​CMakeLists.txt Builds native solver sources.
source/​source_dftb/​test/​test_periodic_scc.cpp Tests periodic SCC behavior. Moderate (3 votes): directly manages MPI lifecycle instead of using project wrappers.
source/​source_dftb/​test/​test_eigensolver.cpp Tests eigensolving and occupations.
source/​source_dftb/​test/​test_charge_mixing.cpp Tests charge mixing.
source/​source_dftb/​test/​CMakeLists.txt Defines DFTB unit tests.
source/​source_dftb/​skf_data.h Declares SKF data structures.
source/​source_dftb/​skf_data.cpp Parses and interpolates SKF data. Critical (1 vote): table-row parsing does not match the bundled SKF sections.
source/​source_dftb/​sk_matrix.h Declares Bloch matrix assembly.
source/​source_dftb/​sk_matrix.cpp Assembles matrices and repulsion. Critical (1 vote): self-image repulsion is counted at half its periodic multiplicity.
source/​source_dftb/​sk_block.h Declares Slater–Koster blocks.
source/​source_dftb/​sk_block.cpp Builds s+p pair blocks.
source/​source_dftb/​periodic_scc.h Declares periodic SCC APIs.
source/​source_dftb/​eigensolver.h Declares generalized eigensolver APIs.
source/​source_dftb/​eigensolver.cpp Implements LAPACK eigensolving and occupations. Moderate (1 vote): zero-weight spectra affect finite-temperature electron counting.
source/​source_dftb/​CMakeLists.txt Defines the DFTB object library.
source/​source_dftb/​charge_mixing.h Declares SCC mixers.
source/​source_dftb/​charge_mixing.cpp Implements linear, Pulay, and Broyden mixing.
source/​source_cell/​read_atoms_helper.cpp Avoids orbital loading for DFTB.
source/​source_cell/​read_atom_species.cpp Skips incompatible orbital metadata.
source/​source_base/​parallel_common.h Declares rank and size helpers.
source/​source_base/​parallel_common.cpp Implements rank and size helpers.
source/​Makefile.Objects Adds Makefile build objects.
source/​CMakeLists.txt Adds DFTB build and link integration. Critical (2 votes): DFTB objects follow numerical providers, risking unresolved LAPACK/BLAS symbols.
docs/​parameters.yaml Updates parameter metadata.
docs/​advanced/​input_files/​input-main.md Documents new input parameters.
docs/​advanced/​input_files/​index.rst Adds DFTB documentation.
docs/​advanced/​input_files/​dftb-native.md Documents native DFTB usage and scope.
.github/​workflows/​test.yml Runs the integration regression.
.github/​workflows/​build_test_cmake.yml Adds native DFTB build/test coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread source/CMakeLists.txt Outdated
{
throw std::invalid_argument("DFTB repulsive pair must have finite, positive length");
}
energy += sk.ab->repulsive.evaluate(distance).energy_hartree;
Comment on lines +395 to +396
// Match DFTB+'s legacy reader, which decrements the file header count once.
const std::size_t n_table_rows = data.declared_grid_points - 1;
for (std::size_t b = 0; b < ntype; ++b)
{
const std::string path = config.skf_directory + separator + labels[a] + "-" + labels[b] + ".skf";
this->skfiles_[a * ntype + b] = ModuleDFTB::SkfData::read_legacy(path, a == b);
Comment thread source/source_dftb/test/test_periodic_scc.cpp Outdated
Comment thread source/source_io/module_parameter/read_inp_sys.cpp

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Review effort: Lite
Findings: 6 High severity

Open (6)
Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Low severity Update INPUT help to document the dftb basis

source/​source_io/​module_parameter/​read_inp_estruc.cpp:22

Rule: User-facing INPUT help must stay consistent with registered parameter values.
Severity: warning
Location: source/source_io/module_parameter/read_inp_estruc.cpp:15-22
Reason: This registration adds the dftb basis, but ParameterHelp::show_general_help() still prints basis_type - Basis set type (pw, lcao) (source/source_io/input_help.cpp:461). A user invoking the general CLI help is therefore told that the newly valid native DFTB basis is unsupported.
Suggested action: update the hard-coded general help (and its coverage if applicable) to mention dftb and the native solver, or generate this summary from the parameter registry.
Exception: not allowed

Comment on lines +664 to +665
Parallel_Reduce::reduce_all(result.spectra[ik].solution.eigenvalues_hartree.data(),
static_cast<int>(n_orbitals));
{
// The heteronuclear metadata line holds legacy polynomial-repulsive
// fields; this stage only uses the Spline repulsive section below.
(void)require_numbers(lines, next++, 20, filename, "heteronuclear repulsive metadata");
this->template_.pair_parameters.clear();
for (std::size_t a = 0; a < ntype; ++a)
{
for (std::size_t b = a; b < ntype; ++b)
@mohanchen mohanchen added the Feature Discussed The features will be discussed first but will not be implemented soon label Sep 30, 2026

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

Feature Discussed The features will be discussed first but will not be implemented soon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants