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

Modification needed for SAWF in Wannier library mode. #342

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

sponce24
Copy link
Contributor

@sponce24 sponce24 commented Sep 2, 2020

The .dmn file needs to be read.

@codecov
Copy link

codecov bot commented Sep 2, 2020

Codecov Report

❗ No coverage uploaded for pull request base (develop@528a67f). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 256b864 differs from pull request most recent head e61ce19. Consider uploading reports for the commit e61ce19 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #342   +/-   ##
==========================================
  Coverage           ?   66.40%           
==========================================
  Files              ?       29           
  Lines              ?    17992           
  Branches           ?        0           
==========================================
  Hits               ?    11948           
  Misses             ?     6044           
  Partials           ?        0           
Impacted Files Coverage Δ
src/disentangle.F90 81.00% <ø> (ø)
src/hamiltonian.F90 48.34% <ø> (ø)
src/io.F90 60.46% <ø> (ø)
src/parameters.F90 83.06% <ø> (ø)
src/plot.F90 49.40% <ø> (ø)
src/postw90/berry.F90 76.27% <ø> (ø)
src/postw90/get_oper.F90 64.58% <ø> (ø)
src/postw90/kpath.F90 63.69% <ø> (ø)
src/postw90/kslice.F90 68.95% <ø> (ø)
src/postw90/postw90_common.F90 68.86% <ø> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 528a67f...e61ce19. Read the comment docs.

I see no reason why SLWF and SLWF+C cannot work for all WF.
It seems to work and give reasonable results.
@hjunlee
Copy link
Contributor

hjunlee commented Sep 4, 2020

Hello, Samuel:

Regarding SLWF, please see the comment at #287 (comment) .
Regarding SAWF, if you have in mind the use of SAWF in EPW, I think that dmn files should be calculated in EPW in order to make sure the gauge consistency.

Sincerely,

Hyungjun Lee

@sponce24
Copy link
Contributor Author

Hello Hyungjun,

For SLWF, it seems to work to constrain all centers (I tested it). The reason that they never constrained all centers is due to their DEFT-based application. Therefore it think it would not hurt to allow it (potentially with a Warning message).

For SAWF, the fix is needed for any code using w90 in library mode.
I also tried using it with EPW indeed.
It also seems to work but I agree it would be better to compute it internally, but this is a different question that we can solve in the EPW repo.

Best,
Samuel

@@ -827,7 +827,7 @@ subroutine param_read()
if (slwf_num .gt. num_wann .or. slwf_num .lt. 1) then
call io_error('Error: slwf_num must be an integer between 1 and num_wann')
end if
if (slwf_num .lt. num_wann) selective_loc = .true.
if (slwf_num .lt. num_wann + 1) selective_loc = .true.
Copy link
Member

Choose a reason for hiding this comment

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

Hi @sponce24 can you explain this change? I think selective_loc is meant to be set if only some of the WFs are selectively localised? @VVitale maybe can also comment

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @sponce24 @giovannipizzi and @hjunlee, apologies for the very late reply. As I mentioned in #287, you should not be able to arbitrarly fix "all" centers of the Wannier functions and expect this to work for an isolate manifold with number of bands = number of WFs, since the vectorial sum of the centers is a gauge invariant quantity (modulo a lattice vector). Whether this is possible for entangled bands, I am not really sure...

@jryates
Copy link
Member

jryates commented Jul 22, 2021

Samuel - apologies for the delay in looking at this PR.

Could you let us know if you would still like this merged (and maybe answer Giovanni's question). If so we will probably merge when we have a coding meeting on the 4th August.

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.

5 participants