msg-byu / enumlib

Derivative structure enumeration library
MIT License
59 stars 34 forks source link

Adding supercell convergence code and fixing bug in makeStr.py #53

Closed wsmorgan closed 6 years ago

glwhart commented 6 years ago

Did all the unit tests pass? Did you add any unit tests for the added parts?

wsmorgan commented 6 years ago

So the python code I changed doesn't have unit tests on this repo, but they pass in phenum. I haven't created a new unit test for the supercell code yet though, I was going to wait until we knew which metric we wanted to use to pick between HNFs with the same maximum rmin so that I won't have to change the unit tests a week from now.

glwhart commented 6 years ago

A good metric for breaking ties would be picking the one with the large point group. That would give better kpoint folding.

wsmorgan commented 6 years ago

But having the best k-point folding might not give you the best spacing between the atom and it's periodic image. I'm thinking we may want to filter ties by minimizing the largest cell vector. Then the smallest and largest vectors would be as similar as possible.

glwhart commented 6 years ago

Yes, but you asked what to do in the case of ties---our first criterion is shortest basis vector distance. If there are ties there, then we could check symmetry.

Since you're already mink reduced when you spit out the list, there's no chance of making any vector shorter.

glwhart commented 6 years ago

Did I misunderstand? You wanted to sort first by shortest vector (what we do now) and then, for ties, sort on the longest vector? Sorry if that's what you meant. We should just make a list for some specific cases and see 1) how often we get ties, 2) what the difference is for point group size among ties, and 3) see if the longest vectors vary when the shortest vector ties.