eri_use_atomics added as option to hamiltonian construction & ecp fix - #583
Open
vbarandun wants to merge 1 commit into
Open
eri_use_atomics added as option to hamiltonian construction & ecp fix#583vbarandun wants to merge 1 commit into
vbarandun wants to merge 1 commit into
Conversation
…ged core energy computation to read from basis set, instead of structure. Will try to add eri_use_atomic to cholesky hamiltonian if theres time in the future.
Contributor
There was a problem hiding this comment.
Pull request overview
Exposes the SCF backend’s eri_use_atomics switch in the Hamiltonian construction path to reduce memory pressure in AO→active-MO ERI transformations, and fixes core-energy (nuclear repulsion) computation for ECP basis sets by using ECP-adjusted nuclear charges from the internal converted molecule.
Changes:
- Add
HamiltonianSettings::eri_use_atomics(defaultfalse) and forward it into the SCF ERI configuration used by Hamiltonian construction. - Compute nuclear repulsion energy from the ECP-adjusted internal molecule (
internal_basis_set->mol) instead ofStructure::calculate_nuclear_repulsion_energy(). - Replace all uses of
structure->calculate_nuclear_repulsion_energy()in the canonical Hamiltonian constructor with the new ECP-adjusted value.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cpp/src/qdk/chemistry/algorithms/microsoft/hamiltonian.hpp | Adds the eri_use_atomics setting to the native Hamiltonian constructor settings. |
| cpp/src/qdk/chemistry/algorithms/microsoft/hamiltonian.cpp | Forwards eri_use_atomics into the ERI backend config and fixes nuclear repulsion energy to use ECP-adjusted nuclear charges. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+206
to
+210
| // ECP-adjusted nuclear repulsion energy. | ||
| // Uses basis set information instead of structure information. | ||
| const double nuclear_repulsion_energy = | ||
| detail::calculate_ecp_adjusted_nuclear_repulsion_energy( | ||
| *internal_basis_set->mol); |
Comment on lines
+21
to
+24
| set_default("eri_use_atomics", false, | ||
| "Use atomic accumulation (one shared buffer) instead of " | ||
| "per-thread private buffers during the AO->active-MO ERI " | ||
| "transformation."); |
Comment on lines
+206
to
+210
| // ECP-adjusted nuclear repulsion energy. | ||
| // Uses basis set information instead of structure information. | ||
| const double nuclear_repulsion_energy = | ||
| detail::calculate_ecp_adjusted_nuclear_repulsion_energy( | ||
| *internal_basis_set->mol); |
Collaborator
|
Hey vbarandun, can you accept the CLA, merge main and run the pre-commit hooks to fix the linting error? |
Author
|
@microsoft-github-policy-service agree company="ETH Zurich" |
Jan Unsleber (nabbelbabbel)
added
bug
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
eri_use_atomicsoption, which caused an OOM kill for large systems when using many OMP threads.scf_solveralready exposes aneri_use_atomicssetting for exactly this purpose. Now exposed asHamiltonianSettings::eri_use_atomics(default false, matchingscf_solver).Structure::calculate_nuclear_repulsion_energy(), whichsums un-ECP-adjusted atomic numbers. This yields wrong numbers when the basis set carries one. Added
detail::calculate_ecp_adjusted_nuclear_repulsion_energy(), mirroring SCFImpl::calc_nuclear_repulsion_energy_(),scf_impl.cpp), computed once from the already-ECP-correctedinternal_basis_set->mol.