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

Add possibility to print bvec. #176

Merged
merged 9 commits into from
Jun 21, 2018

Conversation

sponce24
Copy link
Contributor

  • new input variable to trigger the printing
  • new test

This can then be used by EPW to compute velocities.
A new input variable "write_bvec" has been introduced.
A new test has been introduced.
@codecov
Copy link

codecov bot commented May 20, 2018

Codecov Report

Merging #176 into develop will increase coverage by 0.04%.
The diff coverage is 95%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #176      +/-   ##
===========================================
+ Coverage    57.95%   57.99%   +0.04%     
===========================================
  Files           27       27              
  Lines        16031    16051      +20     
===========================================
+ Hits          9290     9309      +19     
- Misses        6741     6742       +1
Impacted Files Coverage Δ
src/hamiltonian.F90 45.61% <ø> (ø) ⬆️
src/parameters.F90 77.65% <100%> (+0.02%) ⬆️
src/plot.F90 21.11% <94.11%> (+1.44%) ⬆️

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 9af5ed2...7b5e62d. Read the comment docs.

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.

Thanks a lot!
I just added a few comments, plus a couple of additional things: since you are adding a flag, could you add the respective documentation for the flag, and for the format of the output file, in the user guide?

! write_rmn = .true.
! write_bvec = .true.
if (write_bvec) then
open(file_unit,file=trim(seedname)//'.bvec',form='formatted',status='unknown',err=101)
Copy link
Member

Choose a reason for hiding this comment

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

Could you point to a different error message mentioning the .bvec file? (Label 101 instead mentions _r)

test.txt Outdated
@@ -0,0 +1 @@
sdfs
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove this file?

! RM and SP - write to file vectors b and their weight wb for each k-point
! This is used by EPW to compute the velocity.
! Note that you need to put in your Wannier input:
! write_hr = .true.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of requiring all three flags to be set, could you put the code in a different function, and in plot_main (plot.F90) you do, toward the end, below

if(wannier_plot) call plot_wannier

something like

if(write_bvec) call plot_bvec

Note that the file will be changed when merging #177, this automatically also remove the need of write_hr (so you might want to work on top of those changes - in this case, add also the .or. write_bvec at the top, where the header is printed).

num_wann = 4
num_iter = 20

write_hr = .true.
Copy link
Member

Choose a reason for hiding this comment

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

I would make a test where only write_bvec is set, and all other plotting options are removed (write_hr, write_rmn) - see also my other comment above.

[testw90_bvec]
program = WANNIER90_WOUT_OK
inputs_args = ('lead.win', '')
output = lead.wout
Copy link
Member

Choose a reason for hiding this comment

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

I think this test shouldn't check the .wout, but rather we should add a new program and a new parser to check the check the new bvec file (see e.g. what I did in #177 for a different file). You can then check with the parser, e.g., the number of bvecs or even all values.

@giovannipizzi
Copy link
Member

as a note, now #177 has been merged, I updated your branch so you can work directly on that version (see my comments above).

write(file_unit,*) nntot
do nkp = 1, num_kpts
do nn = 1, nntot
write (file_unit,'(4F12.6)') bk(:,nn,nkp), wb(nn)
Copy link
Member

Choose a reason for hiding this comment

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

Another request about the format of the file: could you add two header lines, as in most of the other files:

  1. first line as a comment, including a timestamp, in order to ease debug in case people regenerate many times the file and want to know if it was regenerated or not
  2. Second line with the size of the matrix (two integers, num_kpts and nntot)

You can give a look to other files to see how the header looks like

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.

Thanks a lot @sponce24 !

@giovannipizzi giovannipizzi merged commit 3ee17aa into wannier-developers:develop Jun 21, 2018
giovannipizzi added a commit to giovannipizzi/wannier90 that referenced this pull request Jun 21, 2018
manxkim pushed a commit to manxkim/wannier90 that referenced this pull request Jan 10, 2021
manxkim pushed a commit to manxkim/wannier90 that referenced this pull request Jan 10, 2021
manxkim pushed a commit to manxkim/wannier90 that referenced this pull request Jan 10, 2021
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.

3 participants