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

bugfix: calling w90 as a lib twice does not reinitialize Hamiltonian #224

Merged
merged 3 commits into from
Dec 21, 2018

Conversation

mjv500
Copy link
Contributor

@mjv500 mjv500 commented Dec 21, 2018

abinit call of w90 as a library fails for spin polarized systems, because the destroy function for the hamiltonian was not being reinitialized. Basically the flags are all "public, save" and not reset by the destructor.

When's the release?? I say 25th December around 10:00 AM GMT

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!
Beside the comment in the code, could you also run the precommit code to fix formatting, otherwise the test will not pass? Instructions here: https://github.com/wannier-developers/wannier90/wiki/ContributorsGuide#automatic-pre-commits-spaces-indentation-

For the release, this will be in the second half of January, as we found a couple of bugs that we wanted to fix first.

@@ -235,8 +235,16 @@ subroutine wann_main
if (ierr /= 0) call io_error('Error in allocating cdq in wann_main')

! for MPI
allocate (counts(0:num_nodes - 1), displs(0:num_nodes - 1), stat=ierr)
if (ierr /= 0) call io_error('Error in allocating counts and displs in wann_main')
allocate (counts(0:num_nodes - 1), stat=ierr)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe better to also add a if (allocated(counts)) deallocate(counts) as you do below for displs?

@codecov
Copy link

codecov bot commented Dec 21, 2018

Codecov Report

Merging #224 into develop will increase coverage by 0.02%.
The diff coverage is 88.23%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #224      +/-   ##
===========================================
+ Coverage    62.05%   62.07%   +0.02%     
===========================================
  Files           29       29              
  Lines        17136    17151      +15     
===========================================
+ Hits         10634    10647      +13     
- Misses        6502     6504       +2
Impacted Files Coverage Δ
src/hamiltonian.F90 46.91% <100%> (+1.3%) ⬆️
src/wannierise.F90 81.61% <80%> (-0.04%) ⬇️

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 4555d4b...f5d9ea6. Read the comment docs.

@giovannipizzi giovannipizzi changed the title bugfix: calling w90 as a lib twice does not reinitialize Hamiltonian … bugfix: calling w90 as a lib twice does not reinitialize Hamiltonian Dec 21, 2018
@giovannipizzi giovannipizzi merged commit d4a2455 into wannier-developers:develop Dec 21, 2018
@giovannipizzi
Copy link
Member

Thanks!

@mjv500
Copy link
Contributor Author

mjv500 commented Dec 22, 2018 via email

@giovannipizzi
Copy link
Member

Sure! we'll also advertise on the mailing list. We've done a number of major fixes to the library mode last month to adapt it to the parallelisation of the main code (the library mode is still only serial, though), we've added some tests but I'm pretty sure there are more features that are not tested. So if you find more issues in the next month we'd be happy to get feedback!

@mjv500
Copy link
Contributor Author

mjv500 commented Dec 22, 2018 via email

@giovannipizzi
Copy link
Member

Parallel library is not on the pipeline at the moment - before being able to do it properly, we might need to do a major refactoring of the code (see #2)

manxkim pushed a commit to manxkim/wannier90 that referenced this pull request Jan 10, 2021
bugfix: calling w90 as a lib twice does not reinitialize Hamiltonian
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.

2 participants