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

Support magmoms in get_phonopy_structure() #3555

Merged
merged 4 commits into from
Jan 15, 2024

Conversation

tomdemeyere
Copy link
Contributor

Summary

Phonopy needs magnetic moments to calculate the symmetry etc, currently pymatgen does not send it.

Checklist

  • Google format doc strings added. Check with ruff.
  • Type annotations included. Check with mypy.
  • Tests added for new features/fixes.

python -m pytest tests/io/test_phonopy.py fails with "ImportError: cannot import name 'coord_cython' from 'pymatgen.util'", any ideas?

@JaGeo
Copy link
Member

JaGeo commented Jan 15, 2024

Tagging @JonathanSchmidt1 as I have seen a similar fix in one of his atomate2 branches.

Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tomdemeyere! 👍

@janosh janosh added fix Bug fix PRs symmetry Space groups and the like magmoms Magnetism related phonon Lattice vibrations labels Jan 15, 2024
@JonathanSchmidt1
Copy link
Contributor

@JaGeo My solution basically was the same as here.
Maybe sth relevant for you is that when I added magnetic moments it broke sth in atomate2 because some of the functionalities of phonopy did not work with magnetic moments at the time.
I assume this change in the materials project will lead to the workflow accepting magnetic structures automatically.

 get_phonopy_structure(structure)
 if use_symmetrized_structure == "primitive" and kpath_scheme != "seekpath":
        primitive_matrix: list[list[float]] | str = [
            [1.0, 0.0, 0.0],
            [0.0, 1.0, 0.0],
            [0.0, 0.0, 1.0],
        ]
    else:
        primitive_matrix = "auto"

For me auto was not working with magmoms so I had to remove it. If this is still the case, it might be an option to give a warning in atomate2 and specifically disable auto for magnetic structures.

@JaGeo
Copy link
Member

JaGeo commented Jan 15, 2024

@JonathanSchmidt1 Thanks! I will keep the potential issue for atomate2 in mind.

@janosh janosh merged commit 88921da into materialsproject:master Jan 15, 2024
22 checks passed
@janosh janosh changed the title Add magmom support in get_phonopy_structure Support magmoms in get_phonopy_structure() Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix PRs magmoms Magnetism related phonon Lattice vibrations symmetry Space groups and the like
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants