wannier90 icon indicating copy to clipboard operation
wannier90 copied to clipboard

Parallelization of Wannier functions plotting disappeared in the latest develop branch

Open npaulish opened this issue 1 year ago • 6 comments

As I also mentioned in the email to @JeromeCCP9, even though #370 was merged, wannier_plot is not parallelized in the latest develop branch. Maybe there could be also other PRs that disappeared after merging the library mode? FYI @jiang-yuha0 @qiaojunfeng @giovannipizzi

npaulish avatar May 15 '24 09:05 npaulish

I think #332 could be another victim, see https://github.com/wannier-developers/wannier90/pull/332#issuecomment-2162885492

PeterKraus avatar Jun 12 '24 12:06 PeterKraus

Hi Peter! Thank you for flagging this up, I will track it down...

JeromeCCP9 avatar Jun 12 '24 13:06 JeromeCCP9

Thanks Peter! @JeromeCCP9 I now have a strong feeling that quite a few PRs might have been discarded. We'll have to check several of them, probably in a time range, I imagine from when you started working in the code onwards. It might be that most of them are actually lost, as you worked on top of earlier versions?

giovannipizzi avatar Jun 12 '24 17:06 giovannipizzi

There are I think 34 merged PRs between 3.1 and the merge of the library interface. I would check all. Hopefully about a half should be not on the code (tests, docs,...) so those should be OK. So hopefully there are not that many that could have been lost. But the others might require a bit of work to re implement in the new code structure

giovannipizzi avatar Jun 12 '24 17:06 giovannipizzi

yes, this is true. The corresponding commits exists in the history, but the changes are exactly undone in the current state of develop. I think this means a merge has gone wrong, which is a small nightmare. I investigate more.

JeromeCCP9 avatar Jun 12 '24 17:06 JeromeCCP9

yap, I caused this in merge 6f7859ef8 ; merging the use of types, Feb 2021. Unfortunately it is not a merge commit, so I must have rebased incorrectly. From the date, I can see that this will indeed affect a couple of other PRs.

I'm so sorry about this! I work to resolve it.

JeromeCCP9 avatar Jun 12 '24 18:06 JeromeCCP9