Skip to content

Refactor Hcontainer-related codes: remove current_R in atom_pair.h#7497

Open
mohanchen wants to merge 8 commits into
deepmodeling:developfrom
mohanchen:20260619
Open

Refactor Hcontainer-related codes: remove current_R in atom_pair.h#7497
mohanchen wants to merge 8 commits into
deepmodeling:developfrom
mohanchen:20260619

Conversation

@mohanchen

@mohanchen mohanchen commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

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:

  • Hide underlying invocation logic: Encapsulate the internal strict variable assignment sequence and sequential member function calling logic, shield implicit underlying constraints from external callers.
  • Unify external interface capabilities: Provide highly aggregated and integrated external interfaces, avoid fragmented multi-step manual calls by users, and reduce mental burden.
  • Enhance fault tolerance and robustness: Add necessary state verification and parameter legitimacy checks inside the class to prevent logic errors caused by incorrect calling sequences.
  • Reduce usage costs: Enable all developers to use HContainer correctly and stably without fully understanding the internal implementation details, eliminating exclusive usage barriers for original developers.

@mohanchen mohanchen requested a review from ErjieWu June 20, 2026 01:08
@mohanchen mohanchen added the Refactor Refactor ABACUS codes label Jun 20, 2026
@mohanchen mohanchen changed the title Refactor Hcontainer-related codes Refactor Hcontainer-related codes: remove current_R in atom_pair.h Jun 20, 2026
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 ErjieWu 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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor Refactor ABACUS codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants