-
Notifications
You must be signed in to change notification settings - Fork 15
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
Equivalent points #280
Equivalent points #280
Conversation
Pull Request Test Coverage Report for Build 1049774120
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just briefly going over, I have to admit that I don't understand the uses cases get_symmetry()
vs. get_equivalent_point()
vs. group_points_by_symmetry()
. I'll try to have a detailed look again tomorrow, but I do feel this is something the docstring should tell me without reading the source.
Co-authored-by: Marvin Poul <[email protected]>
Most important changes
symmetry.py
(which allowed me to reduce 200 lines inatoms.py
)symmetry = structure.get_symmetry()
, after which the attributes ofsymmetry
can be used for different purposed (in the end this follows @jan-janssen's suggestion)structure.get_symmetry()
is still available (i.e.structure.get_symmetry()['rotations']
etc.)Original purpose of this PR (i.e.
get_arg_equivalent_sites()
)This is very much equivalent to
structure.group_points_by_symmetry
, although the old function didn't work in many cases. Also instead of grouping the sites by lists, the new function returns their ID's.Example: Group octahedral interstitial sites according to the box symmetries:
This returns:
This should be very much equivalent to the distances between the vacancy and these positions:
Comments
symmetry._get_spglib_cell()
. However, for some reasonstructure.get_cell().T
was used for most of the places except forget_spacegroup()
(which is now a property,spacegroup
). As I have the feeling that.T
should be removed everywhere, I removed it -> if you know more about this, and if this presents a problem, please let me know.