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

Move SCDM flags to pw2wannier90 #238

Merged

Conversation

VVitale
Copy link
Collaborator

@VVitale VVitale commented Feb 8, 2019

This fixes #237.
Moving the four SCDM flags from parameters.F90 to pw2wannier.F90 (now in pwscf/POST-v6.3-with-SCDM folder).
No scdm_info block is written into .nnkp files for back-compatibility.
New behaviour:

  1. If there is a projection block in the .win file then the projection block in the .nnkp file remains the same . In this case if scdm_proj = T in the .pw2wan input file the code uses the SCDM method with the given number of projections. A warning is fired both in the pw2wannier90 output file and wannier90 output file to make the user aware of this. The warning also provides some suggestions on how to change the input files if this is not the intended behaviour.
  2. If no projection block is present in the wannier90 input file, then the projection block in the .nnkp file looks like this
begin projections
     0     num_wann
end projections

Where num_wann is the number of projections to use in the SCDM. I have code this in such a way that doesn't break back compatibility.

I have also modified kmesh.F90 such as no scdm_info block is written into .nnkp files; overlap.F90 in order to read the new header in the .amn files and set the number of projections accordingly.

I have also made changes to the input files of example27 and modified the tutorial. I have removed the description of the scdm keywords from the user guide but I have left the section describing the SCDM method.

@VVitale VVitale changed the title Move scdm to pw2wannier90 Move scdm to pw2wannier90. Fixes Feb 8, 2019
@VVitale VVitale changed the title Move scdm to pw2wannier90. Fixes Move scdm to pw2wannier90. Fixes #237 Feb 8, 2019
@codecov
Copy link

codecov bot commented Feb 8, 2019

Codecov Report

Merging #238 into develop will increase coverage by 0.02%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #238      +/-   ##
===========================================
+ Coverage    62.01%   62.04%   +0.02%     
===========================================
  Files           29       29              
  Lines        17161    17132      -29     
===========================================
- Hits         10643    10630      -13     
+ Misses        6518     6502      -16
Impacted Files Coverage Δ
src/overlap.F90 76.61% <100%> (ø) ⬆️
src/kmesh.F90 63.78% <25%> (-0.14%) ⬇️
src/parameters.F90 81.75% <85.71%> (+0.31%) ⬆️

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 d8f99c9...6a3d7b8. Read the comment docs.

@VVitale
Copy link
Collaborator Author

VVitale commented Feb 8, 2019

@giovannipizzi has made the following suggestion to make the code fully back-compatible and also in a certain sense forward-compatible with the Mustafa implementation (selection of fewer projections):
We write a block in the .nnkp code with the following structure

begin partial_projections
  tot_num_wannier
  0 # or number of following lines 
  # every line is an index of the projection in the projections block to use for partially-automated
end partial_projections

or something similar. In this way we pass the info about the number of projections to the SCDM in back compatible way. This block is only written if a specific new flag is T in the wannier90 input (e.g. use_auto_proj), so that wannier90 "knows" that the SCDM has been used through an internal flag and not only through the header of the .amn file (which is not very safe). We then have four cases that we can handle safely and the logic can be extended to future implementations when fewer projections than the number specified in the projection block are selected.
@mostofi @jryates what do you think? Do you see any flaw in this logic? Do you have any preference for the name of the new flag and the block in .nnkp?

@VVitale VVitale self-assigned this Feb 8, 2019
@giovannipizzi giovannipizzi changed the title Move scdm to pw2wannier90. Fixes #237 Move SCDM flags to pw2wannier90 Feb 22, 2019
@giovannipizzi
Copy link
Member

After discussion, we'll call the flag in Wannier90 auto_projections (boolean) and put a block begin auto_projections in the nnkp file, only if the flag is true. @VVitale will implement correct warning/failure logic depending on the combination of flags both in Wannier90 and pw2annnier90, to minimise the risk of a user thinking to do something but actually computing something else.

Copy link
Member

@giovannipizzi giovannipizzi left a comment

Choose a reason for hiding this comment

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

All very good, thanks! I'm approving and merging this even if also the files.tex should also be updated (nnkp file). I will open an issue for this.
In this way we can already test this new functionality (I will test on QE 6.3). If there are bugs we'll make a new PR.

@giovannipizzi giovannipizzi merged commit 2ab7a90 into wannier-developers:develop Feb 24, 2019
manxkim pushed a commit to manxkim/wannier90 that referenced this pull request Jan 10, 2021
…w2wannier90

Move SCDM flags to pw2wannier90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move SCDM input keywords from Wannier90 to pw2wannier90?
2 participants