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

Fix 1028 structure to pymagen #1285

Merged
merged 17 commits into from
Mar 15, 2018

Conversation

nmounet
Copy link

@nmounet nmounet commented Mar 15, 2018

Fix for #1028 (for pymatgen).

Now StructureData -> pymatgen -> StructureData roundtrip works with all kinds of kind names, using the 'properties' of pymatgen structure sites. It works even with letters and strings (and not only with ordinals appended to the symbol, as for ASE)
In addition, a number of tests have been added (including for ASE), to test in particular roundtrip behaviour, or conversion to/from pymatgen structure with partial occupancies.
Finally, StructureData -> pymatgen conversion (and vice-versa) now supports spins (only for structure, not for molecule), using the rule that kind names ending with 1 indicate spin +1, and kind name ending with 2 are for spin -1.
To trigger the conversion to a pymatgen structure with spin, one simply needs to do
structure.get_pymatgen(add_spin=True)

Note that partial occupancies plus spin and not supported for these conversions; tests have been created to check than an exception is raised when the user tries to do such a conversion.

Nicolas Mounet added 14 commits March 14, 2018 11:52
…gen is impossible when both spins and partial occupancies are used
… when structures with both spins and partial occupancies are tentatively converted
…o ensure correct transferring back-and-forth of kind names
…to all pymatgen sites, whenever there are customized kind names
…en sites, are now converted to StructureData kind names
…sing the additional kwarg add_spin; fix the corresponding tests
@giovannipizzi
Copy link
Member

Awesome! For the failing test, it's a known issue and @szoupanos is going to commit a fix in develop, so merging develop in after his PR should solve the problem.

giovannipizzi
giovannipizzi previously approved these changes Mar 15, 2018
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