Closed lmhale99 closed 2 years ago
Hi @lmhale99! Thank you for your further testing and consideration. I am afraid that I have not fully understood your new method yet, but let me comment for the moment.
I tried to figure out what your method was doing, but I wasn't entirely sure so I tried doing my own (unique_shifts2)
Sorry, I should leave more comments or documents since symmetry operations on crystal planes are a bit cumbersome. Here is my implementation note. I hope this may clarify my first attempt.
I took what I do in wrap to avoid the need of the trial image search.
Is original unique_shifts
failing to find distinct crystal planes due to the trial image search?
I will work on investigating in which cases unique_shifts
fails.
Alternatively, I think the most complete method would be to take all of the rcell + shift systems and compare the atomic positions in those systems for equivalency.
I agree on although this approach may be time-consuming. This approach seems to be used in pymatgen’s surface module with StructureMatcher.
By the way, I think requirements.txt
needs to be updated to include yabadaba==0.1.1
and the newest version of potentials
to work this PR.
@lan496,
I created a new branch for unique-shifts as my tests show that it is not working as well as I think it should. When I loop over hkl sets with different prototypes (https://github.com/lmhale99/potentials-library/tree/master/free_surface) I see that the unique_shifts method still returns a large number of shifts despite the fact that most or all should be the same.
Small updates to your method:
I tried to figure out what your method was doing, but I wasn't entirely sure so I tried doing my own (unique_shifts2)
Maybe look over the two methods and see if you can see how to improve either.
Alternatively, I think the most complete method would be to take all of the rcell + shift systems and compare the atomic positions in those systems for equivalency.
I also added code for a system dump style that uses spglib to return a primitive unit cell. While the code works if copied somewhere else, it is not incorporated into atomman as doing so would create recursive imports at the moment...