Skip to content

Fix(input): reject esolver_type=lr with a self-consistent calculation - #7779

Open
Stardust0831 wants to merge 1 commit into
deepmodeling:developfrom
Stardust0831:fix/input-lr-requires-nscf
Open

Fix(input): reject esolver_type=lr with a self-consistent calculation#7779
Stardust0831 wants to merge 1 commit into
deepmodeling:developfrom
Stardust0831:fix/input-lr-requires-nscf

Conversation

@Stardust0831

@Stardust0831 Stardust0831 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

esolver_type=lr ("LR-TDDFT with given KS orbitals") reads the ground state wave function (and charge density) from a separate, prior SCF run, so it is only meaningful as a non-self-consistent step. It cannot be combined with calculation=scf.

Previously the reset hook in read_input_item_system.cpp silently rewrote calculation from scf to nscf, hiding a contradictory user input and leaving a silently-mutated INPUT state. This is exactly the behavior that issue #7719 asks to eliminate ("A user-specified value should not be silently changed unless that behavior is deliberate, documented, and tested").

This PR removes the silent rewrite and rejects the contradictory combination in check_value with an explicit diagnostic that also states the required value (calculation=nscf), consistent with the approach already adopted for noncolin/lspinorbnspin and bndpar in #7731.

Changes

  • source/source_io/module_parameter/read_input_item_system.cpp
    • Remove the esolver_type reset_value hook that rewrote calculation=scf → nscf.
    • Add a check_value rule that WARNING_QUITs when esolver_type=lr && calculation=scf, with a message that names the required value and explains why.
  • Tests
    • read_input_test.cpp: new ValidateLrRequiresNscf end-to-end test — rejects the default (esolver_type lr alone) and the explicit contradiction (esolver_type lr + calculation scf), and accepts the valid combination (esolver_type lr + calculation nscf).
    • read_input_item_test.cpp: update the Item_test esolver_type block to assert the new check_value rejection instead of the removed reset-to-nscf behavior.

Verification

  • Per ABACUS governance, INPUT behavior changes were verified against the CLI/manual path where possible. The new rule is covered by the unit tests added/modified above; check_value is invoked in the final validation phase after all inputs are read (see read_input.cpp).
  • No parameters.yaml / input-main.md regeneration is required: this change does not modify any parameter's description, default_value, or legal values; it only adds a cross-parameter validation, which is not represented in the generated documentation.

Related

Notes

esolver_type=ks-lr is unaffected: it performs its own KS ground state SCF internally and therefore does not require calculation=nscf.

esolver_type=lr reads the ground state wave function from a separate SCF
run, so it cannot be combined with calculation=scf. Previously the reset
hook silently rewrote calculation to nscf, hiding an invalid user input.

Remove the reset hook and reject the contradictory combination in
check_value with an explicit diagnostic that states the required value.
@Stardust0831
Stardust0831 marked this pull request as ready for review August 5, 2026 11:38
@Growl1234

Copy link
Copy Markdown

I'm surprised that CI tests fails due to PEXSI, given that Docker build has succeed and the PEXSI32_ROOT is even included again in CMAKE_PREFIX_PATH.

@Stardust0831

Stardust0831 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

@Growl1234 Thanks for checking. This failure is unrelated to this PR and was caused by the mutable Intel container tag being overwritten by the LTS branch: the same job succeeded at 09:59 UTC with abacus-intel:latest digest sha256:c38db3f... (IntelLLVM 2025.2.1, with the PEXSI environment exported), then the LTS Container workflow pushed digest sha256:815bcab... to that same latest tag at 10:15 UTC, and the next run failed with IntelLLVM 2026.1.1 because that LTS image does not contain/export PEXSIConfig.cmake or PEXSI32_ROOT. The prepend_path CMAKE_PREFIX_PATH ${PEXSI32_ROOT}:... line shown in the Actions log is the shell command text printed before execution; it does not prove that PEXSI32_ROOT has a value, and in the overwritten image it expands to an empty entry. The LTS Docker build can still succeed because it does not enable the develop-only PEXSI build check. The full timeline and proposed separation of latest and lts-latest are documented in #7780.

@Growl1234

Copy link
Copy Markdown

Uh it's really unfortunate...

@mohanchen mohanchen added Refactor Refactor ABACUS codes Input&Output Suitable for coders without knowing too many DFT details labels Aug 5, 2026
@mohanchen
mohanchen requested review from maki49 and mohanchen August 5, 2026 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Input&Output Suitable for coders without knowing too many DFT details Refactor Refactor ABACUS codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants