Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor istate_charge.cpp to reduce dependence on other modules #4086

Merged

Conversation

AsTonyshment
Copy link
Collaborator

@AsTonyshment AsTonyshment commented May 4, 2024

Linked Issue

Fix #4085.

What's changed?

This pull request introduces a series of refactoring and improvements aimed at enhancing code readability, performance, and modularity. Here are the major changes made:

  1. Used parse_expression Template Function:

    • Overloaded the parse_expression template function to support direct parsing to int types. This update eliminates the need for explicit type conversions previously present, reducing potential type conversion errors.
  2. Isolation of bands_to_print to out_band_kb Conversion:

    • Extracted the conversion from bands_to_print to out_band_kb into a separate function. This change ensures that the process of obtaining out_band_kb is handled internally within the INPUT class, making the intermediate transformation process invisible and inaccessible from outside.
  3. Reduced Dependency on pelec:

    • Modified the system to allow passing wg directly into functions where pelec was previously required. This reduction in dependency decreases coupling across the codebase.
  4. Extended Functionality in efermi Struct:

    • Added a new member function, get_all_ef, to the efermi struct, which returns a vector of Fermi energies for all spins. This enhancement provides a more flexible and straightforward way to access all Fermi energy values across different spins.
  5. Expansion of rhopw Parameters:

    • Decomposed the rhopw parameter into six distinct input parameters.
  6. Expanded bigpw Parameters:

    • Expanded the bigpw parameter into two separate input parameters.

Reminder

  • The decision to maintain the dependency on Local Orbital Charge (LOC) is due to the extensive use of its numerous member functions, which are currently tightly integrated into various parts of the codebase. Detaching this dependency now would be inefficient and could potentially destabilize critical functionalities.

Future Plans for Refactoring idmatrix:

@AsTonyshment AsTonyshment requested a review from mohanchen May 4, 2024 03:25
Copy link
Collaborator

@mohanchen mohanchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mohanchen mohanchen merged commit efb67a9 into deepmodeling:develop May 4, 2024
12 checks passed
@AsTonyshment AsTonyshment deleted the refactor_istate_charge branch May 4, 2024 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need to get rid of some module dependence in nbands_istate
2 participants