Skip to content

Fix silent skip of single-precision PW onsite corrections - #8231

Draft
lanshuyue wants to merge 1 commit into
deepmodeling:developfrom
lanshuyue:codex/fix-pw-onsite-single-precision
Draft

lanshuyue wants to merge 1 commit into
deepmodeling:developfrom
lanshuyue:codex/fix-pw-onsite-single-precision

Conversation

@lanshuyue

Copy link
Copy Markdown
Collaborator

Reminder

  • I read AGENTS.md and docs/developers_guide/agent_governance.md.
  • No linked issue: this failure mode was identified during a focused code review.
  • Focused tests and PW case checks are listed below.
  • User-visible INPUT behavior and core-module impact are described below.
  • No governance exception is requested.

Linked Issue

No linked issue; this PR addresses the single-precision PW onsite silent-skip path found during review.

Unit Tests and/or Case Tests for my changes

  • cmake --build build --target MODULE_IO_read_item_serial abacus_basic_para -j 4 — passed.
  • cmake --build build --target module_pwdft -j 4 — passed.
  • cmake --build build --target onsite_op_test MODULE_PW_dftu_base_test MODULE_PW_test_dftu_pw_tools -j 4 — passed.
  • OMP_NUM_THREADS=1 ctest --test-dir build --output-on-failure -R '^(onsite_op_test|MODULE_PW_dftu_base_test|MODULE_PW_test_dftu_pw_tools|MODULE_IO_read_item_serial)$' — 4/4 passed outside the restricted sandbox. The first sandboxed run could not initialize MPI; the rerun passed.
  • build/abacus_basic_para --version — v3.11.0-beta11; build/abacus_basic_para -h precision — displays the new restriction.
  • OMP_NUM_THREADS=1 build/abacus_basic_para --check-input on isolated copies of PW DFT+U and DeltaSpin cases with precision single — both exited 1 with feature-specific errors before SCF.
  • Double-precision case checks in /tmp: relax_dftu_spin2 (1 rank) and scf_deltaspin2 (2 MPI ranks) completed. DeltaSpin energy, force, and stress match result.ref. The DFT+U relax values differ from the repository reference by 0.0004037623 eV, 0.002652 force sum, and 0.602177 stress sum. The unmodified upstream/develop code, rebuilt and run on the same isolated input, produced exactly the same values as this patch, so there is no numerical drift from this change.
  • python3 tools/03_code_analysis/agent_governance_check.py --staged — no errors; two review warnings explained below.
  • git diff --cached --check — passed.
  • python3 tools/03_code_analysis/code_quality_score.py --format json source/source_pw/module_pwdft/op_pw_proj.cpp — 79 before, 91 after. This is static quality evidence, not numerical validation.

What's changed?

  • Reject single-precision PW Kohn–Sham or TDDFT input when DFT+U or DeltaSpin onsite corrections are enabled, instead of silently applying an empty float operator.
  • Pass nspin to OnsiteProj at construction and use it for the existing nspin=1/2/4 potential layout and spin selection. No double-precision arithmetic or kernel was changed.
  • Add focused input regression coverage. The unused pointer-copy constructor and existing float stubs remain untouched to keep this fix narrow.

Governance Notes

  • INPUT/docs changes: the precision Input_Item help text now states the restriction. Generated documentation is a build artifact and was not committed.
  • Core module impact: the PW Hamiltonian passes its existing PARAM.inp.nspin value once into the operator. Four global reads are removed from op_pw_proj.cpp; the PR has a net reduction of three global references. The remaining read belongs at the existing Hamiltonian construction boundary.
  • CPU/GPU coverage: CPU build and tests passed. GPU/CUDA/ROCm kernels and runtime were not executed because this build is CPU-only. No float onsite kernel support is claimed.
  • Exceptions requested: none.

@mohanchen mohanchen added the Bugs Bugs that only solvable with sufficient knowledge of DFT label Oct 29, 2026

@ABACUS-PR-BOT ABACUS-PR-BOT 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.

No new correctness defect found in the inspected precision guard and nspin handoff. One nonblocking diagnostic suggestion is inline. This was a static review of 7df13fa89513e8227924262bf1541a46c987b14e against e5ad53046d4f4c4502bd04657b7fbeba60d97ad4; builds, tests, CLI and numerical checks were not run, and CI diagnostics were unavailable.

item.check_value = [](const Input_Item& item, const Parameter& para) {
const Input_para& inp = para.inp;
const bool pw_onsite_solver = inp.basis_type == "pw"
&& (inp.esolver_type == "ksdft" || inp.esolver_type == "tddft");

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.

Non-blocking: For esolver_type=tddft, switching to double precision still leaves DFT+U and spin constraints unsupported (pw::check_td_input). Please identify that solver restriction in this diagnostic while preserving early rejection.

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

Bugs Bugs that only solvable with sufficient knowledge of DFT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants