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

Refactoring & Optimization: Removal of GlobalC::UOT and GlobalC::ORB #4079

Closed
8 tasks
jinzx10 opened this issue Apr 30, 2024 · 0 comments
Closed
8 tasks

Refactoring & Optimization: Removal of GlobalC::UOT and GlobalC::ORB #4079

jinzx10 opened this issue Apr 30, 2024 · 0 comments
Assignees
Labels
The Absolute Zero Reduce the "entropy" of the code to 0

Comments

@jinzx10
Copy link
Collaborator

jinzx10 commented Apr 30, 2024

Describe the Code Quality Issue

Non-const global variables introduces hidden dependencies between modules and leads to less structured code. The abuse of such variables almost always damages modularity and makes it difficult to perform unit testing. While such variables may bring a little convenience in some scenarios, in the long run, however, it often turns out that this little benefit cannot compensate for the extra effort of keeping track of where/how those variables are updated, or debugging unintended/unexpected changes of their states. Moreover, unnecessary dependencies make unit tests more tedious than they ought to be.

GlobalC::ORB and GlobalC::UOT are two global variables widespread in the LCAO code. As with many other global varibles, they are used more often than needed. Take ORB as an example. As a collection of numerical radial functions of atomic orbitals, ORB contains all the relevant information, including cutoff radii, radial functions in k-space, values on a finer grid for interpolation, number of radial functions per atomic type, etc. A widespread practice in the present LCAO code is to retretive atomic orbital information from ORB instead of pass function argument. While doing so indeed shortens the argument list, this little benefit is often overwhelmed by its negative impact - a function that merely uses some cutoff radii has to mock/build a whole collection of radial functions in its unit test.

To enforce better modularity, current LCAO code design, which treats these two variables as globally accessible & modifiable objects, should be refactored. Most importantly, classes that do not rely on the major functionalities of ORB/UOT (or their equivalence in module_nao) should no longer couple to them at all. For example, if a class/function merely requires the cutoff radii of all atomic types, passing a std::vector would suffice; if a class merely interpolates the numerical radial functions in ORB, passing some general interpolant objects as const object would be more appropriate.

Details of the refactoring & optimization plan will be updated later.

Additional Context

No response

Task list for Issue attackers (only for developers)

  • Identify the specific code file or section with the code quality issue.
  • Investigate the issue and determine the root cause.
  • Research best practices and potential solutions for the identified issue.
  • Refactor the code to improve code quality, following the suggested solution.
  • Ensure the refactored code adheres to the project's coding standards.
  • Test the refactored code to ensure it functions as expected.
  • Update any relevant documentation, if necessary.
  • Submit a pull request with the refactored code and a description of the changes made.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
The Absolute Zero Reduce the "entropy" of the code to 0
Projects
None yet
Development

No branches or pull requests

2 participants