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

Feature: Support calculation of multi-k point band-decomposed charge density under LCAO basis. #3572

Closed
14 tasks
AsTonyshment opened this issue Jan 29, 2024 · 6 comments · Fixed by #4698
Closed
14 tasks
Assignees
Labels
Input&Output Suitable for coders without knowing too many DFT details

Comments

@AsTonyshment
Copy link
Collaborator

AsTonyshment commented Jan 29, 2024

Background

Currently, ABACUS only supports multi-k point calculation of band-decomposed charge density under PW basis, while under LCAO basis the calculation can be carried out only when gamma_only is set to be true. It is necessary to support calculation of multi-k point charge density under LCAO basis when INPUT keyword is set as calculation = get_pchg.

Describe the solution you'd like

Changes of INPUT keywords:

  1. Delete INPUT keyword nbands_istate.
  2. Add a new INPUT parameter bands_to_print(similar to ocp_set), which enables users to choose which band to calculate.
  3. Add a new INPUT boolean parameter out_all_kpt, which enables users to choose whether to calculate all k-points or gamma point only.

Possible changes of source code: source/module_io/istate_charge.cpp. Currently the calculation of density matrix under LCAO basis set is still using old-fashioned data structure. We plan to rewrite these codes with new density matrix using HContainer (see source/module_elecstate/module_dm/density_matrix.cpp).

Task list only for developers

  • Notice possible changes of behavior
  • Explain the changes of codes in core modules of ESolver, HSolver, ElecState, Hamilt, Operator or Psi

Notice Possible Changes of Behavior (Reminder only for developers)

No response

Notice any changes of core modules (Reminder only for developers)

Only source code in module_io will be changed. See solution description.

Notice Possible Changes of Core Modules (Reminder only for developers)

No response

Additional Context

For a more detailed description (formulae etc.), see Feishu doc https://xmywuqhxb0.feishu.cn/docx/FBHldIvHxoP99axSZeLcHIenntg. One can ask @AsTonyshment for access permission.

Task list for Issue attackers (only for developers)

  • Review and understand the proposed feature and its importance.
  • Research on the existing solutions and relevant research articles/resources.
  • Discuss with the team to evaluate the feasibility of implementing the feature.
  • Create a design document outlining the proposed solution and implementation details.
  • Get feedback from the team on the design document.
  • Develop the feature following the agreed design.
  • Write unit tests and integration tests for the feature.
  • Update the documentation to include the new feature.
  • Perform code review and address any issues.
  • Merge the feature into the main branch.
  • Monitor for any issues or bugs reported by users after the feature is released.
  • Address any issues or bugs reported by users and continuously improve the feature.
@AsTonyshment AsTonyshment added the Feature Discussed The features will be discussed first but will not be implemented soon label Jan 29, 2024
@AsTonyshment AsTonyshment self-assigned this Jan 29, 2024
@WHUweiqingzhou
Copy link
Collaborator

WHUweiqingzhou commented Jan 30, 2024

Look forward to it!
b.t.w, there is a similar PR #3501 for PW basis recently.

@WHUweiqingzhou
Copy link
Collaborator

WHUweiqingzhou commented Jan 30, 2024

And This issue will be close as Issue #3266

@WHUweiqingzhou
Copy link
Collaborator

@AsTonyshment can we close this issue now?

@AsTonyshment
Copy link
Collaborator Author

The implementation for multi-k is not yet complete and must wait for other refactoring tasks to be finished (for example, getting rid of class LocalOrbitalCharge).

@mohanchen mohanchen removed the Plans label Jun 29, 2024
@hongriTianqi
Copy link

@AsTonyshment LOC has been removed. Would you take care of this issue in near future?

@AsTonyshment
Copy link
Collaborator Author

@hongriTianqi This has already been implemented, as you may have just seen in the Feishu doc (which includes a detailed benchmark with VASP). The PR will follow next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Input&Output Suitable for coders without knowing too many DFT details
Projects
None yet
4 participants