Refactor Hcontainer-related codes: remove current_R in atom_pair.h#7497
Open
mohanchen wants to merge 8 commits into
Open
Refactor Hcontainer-related codes: remove current_R in atom_pair.h#7497mohanchen wants to merge 8 commits into
mohanchen wants to merge 8 commits into
Conversation
The PR 4a201ad removed the current_R mutable member from AtomPair, but add_from_matrix was incorrectly rewritten to use values[0] instead of values[current_R]. As a result, when updating the density matrix for non-zero R vectors in DeePKS_domain::update_dmr, the data was always written into the (0,0,0) R block, leaving other R blocks empty. This caused the nspin=2 DeePKS SCF test to fail with a large deviation in deepks_desc and deepks_dm_eig. Add thread-safe add_from_matrix overloads that take an explicit R_index and use values[R_index], and call them in update_dmr via the r_index returned by find_R.
ErjieWu
approved these changes
Jun 21, 2026
Collaborator
There was a problem hiding this comment.
This PR has removed the variable current_R from the internal part of HContainer class which is used to mark the Bravais vector of the current operation position. It has controlled all operations involving the Bravais vector through function parameters passed as arguments, which effectively avoids unnecessary variable dependencies and also enhances the stability during parallel processing.
LGTM.
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.
Refactor codes related to HContainer: remove current_R in atom_pair.h, as this mutable variable introduces potential OpenMP data races.
HContainer Refactoring Requirements
1. Current Problems
The HContainer class provides a set of member functions for external business calls. In actual usage scenarios, these functions often need to be invoked consecutively to complete a complete business operation. However, there are strict mandatory dependencies on the assignment order of internal variables between these sequential member functions.
Currently, such invocation constraints and variable assignment preconditions are only implicitly coupled inside the class, without explicit interface constraints, parameter checks, or unified process encapsulation. This leads to an extremely high threshold for external developers to use HContainer correctly. Only the original developer who implemented the internal logic of this class can fully follow the implicit execution order and avoid hidden pitfalls.
For other team members who call the HContainer interface, it is very easy to make mistakes such as disordered function invocation order and missing prerequisite variable assignments. These incorrect usages will cause abnormal business logic, inconsistent data status, and hard-to-reproduce hidden bugs, which greatly increases the difficulty of troubleshooting and subsequent maintenance costs.
2. Refactoring Objectives
The core goal of HContainer refactoring is to optimize interface usability and robustness, eliminate implicit calling dependencies, and make the component more developer-friendly: