Skip to content

Fix: Fix ELF for kpar > 1#7307

Open
sunliang98 wants to merge 4 commits intodeepmodeling:developfrom
sunliang98:fix_elf
Open

Fix: Fix ELF for kpar > 1#7307
sunliang98 wants to merge 4 commits intodeepmodeling:developfrom
sunliang98:fix_elf

Conversation

@sunliang98
Copy link
Copy Markdown
Collaborator

@sunliang98 sunliang98 commented May 3, 2026

Reminder

  • Have you linked an issue with this pull request?
  • Have you added adequate unit tests and/or case tests for your pull request?
  • Have you noticed possible changes of behavior below or in the linked issue?
  • Have you explained the changes of codes in core modules of ESolver, HSolver, ElecState, Hamilt, Operator or Psi? (ignore if not applicable)

Linked Issue

Fix #7305

Unit Tests and/or Case Tests for my changes

What's changed?

Problem
When running with kpar > 1, ELF output was incorrect. The root cause was that cal_tau() called rho_mpi() again, which re‑reduced rho across k‑point pools and effectively scaled the density by kpar. Since tau_vw depends on $|\nabla \sqrt{\rho}|^2$, this led to incorrect ELF.

Fix
Added Charge::kin_r_mpi() to reduce only kin_r across kpar/bndpar, and updated ElecStatePW::cal_tau() to call this new function. This avoids re‑reducing rho while keeping kin_r properly reduced.

Validation
ELF outputs computed with the updated code are consistent for kpar=1/4.
image

Any changes of core modules? (ignore if not applicable)

  • Example: I have added a new virtual function in the esolver base class in order to ...

Copilot AI review requested due to automatic review settings May 3, 2026 16:13
@sunliang98 sunliang98 requested a review from mohanchen May 3, 2026 16:14
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes incorrect ELF output when running with kpar > 1 by preventing an unintended second reduction of rho during tau/ELF preparation, and instead reducing only kin_r across k-point/band pools.

Changes:

  • Added Charge::kin_r_mpi() to MPI-reduce only kin_r across k-point/band parallel pools.
  • Updated ElecStatePW::cal_tau() to call kin_r_mpi() instead of the broader parallelK()/rho_mpi() path.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
source/source_estate/module_charge/charge_mpi.cpp Adds Charge::kin_r_mpi() to reduce only kinetic energy density (kin_r) across pools.
source/source_estate/module_charge/charge.h Declares the new kin_r_mpi() API on Charge.
source/source_estate/elecstate_pw_cal_tau.cpp Switches cal_tau() to reduce only kin_r, avoiding re-reducing rho.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/source_estate/elecstate_pw_cal_tau.cpp
Comment thread source/source_estate/module_charge/charge_mpi.cpp
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.

Electron localization function (ELF) results in high-pressure liquid Li seems to be way too localized in PBE

2 participants