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 floating point imprecision error in ordering property of CollinearMagneticStructureAnalyzer #3574

Merged
merged 6 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions pymatgen/analysis/magnetism/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -481,10 +481,15 @@ def number_of_unique_magnetic_sites(self, symprec: float = 1e-3, angle_tolerance
@property
def ordering(self) -> Ordering:
"""Applies heuristics to return a magnetic ordering for a collinear
magnetic structure. Result is not guaranteed for correctness.
magnetic structure. Result is not guaranteed to be correct, just a best
guess. Tolerance for minimum total magnetization to be considered
ferro/ferrimagnetic is 1e-8.
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if we should make this adaptable when defining the instance the class if we are not sure about the tolerance yet.

Copy link
Member

Choose a reason for hiding this comment

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

ah, wanted to add that but forgot. PRs welcome!

Copy link
Member

Choose a reason for hiding this comment

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

@kaueltzen will hopefully open another PR as it was one of the questions in the PR!


Returns:
Ordering: Enum ('FiM' is used as the abbreviation for ferrimagnetic)
Ordering: Enum with values FM: ferromagnetic, FiM: ferrimagnetic,
AFM: antiferromagnetic, NM: non-magnetic or Unknown. Unknown is
returned if magnetic moments are not defined or structure is not collinear
(in which case a warning is issued).
"""
if not self.is_collinear:
warnings.warn("Detecting ordering in non-collinear structures not yet implemented.")
Expand All @@ -503,9 +508,9 @@ def ordering(self) -> Ordering:

is_potentially_ferromagnetic = np.all(magmoms >= 0) or np.all(magmoms <= 0)

if total_magnetization > 0 and is_potentially_ferromagnetic:
if abs(total_magnetization) > 1e-8 and is_potentially_ferromagnetic:
return Ordering.FM
if total_magnetization > 0:
if abs(total_magnetization) > 1e-8:
return Ordering.FiM
if max_magmom > 0:
return Ordering.AFM
Expand Down
34 changes: 21 additions & 13 deletions tests/analysis/magnetism/test_analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ def setUp(self):

self.NiO_expt = Structure.from_file(f"{TEST_FILES_DIR}/magnetic.example.NiO.mcif", primitive=True)

# CuO.mcif sourced from https://www.cryst.ehu.es/magndata/index.php?index=1.62
# doi: 10.1088/0022-3719/21/15/023
self.CuO_expt = Structure.from_file(f"{TEST_FILES_DIR}/magnetic.example.CuO.mcif.gz", primitive=True)

lattice = Lattice.cubic(4.17)
species = ["Ni", "O"]
coords = [[0, 0, 0], [0.5, 0.5, 0.5]]
Expand Down Expand Up @@ -161,28 +165,32 @@ def test_get_ferromagnetic_structure(self):
assert CollinearMagneticStructureAnalyzer(s1).matches_ordering(s2_prim)

def test_magnetic_properties(self):
msa = CollinearMagneticStructureAnalyzer(self.GdB4)
assert not msa.is_collinear
mag_struct_analyzer = CollinearMagneticStructureAnalyzer(self.GdB4)
assert not mag_struct_analyzer.is_collinear

msa = CollinearMagneticStructureAnalyzer(self.Fe)
assert not msa.is_magnetic
mag_struct_analyzer = CollinearMagneticStructureAnalyzer(self.Fe)
assert not mag_struct_analyzer.is_magnetic

self.Fe.add_site_property("magmom", [5])

msa = CollinearMagneticStructureAnalyzer(self.Fe)
assert msa.is_magnetic
assert msa.is_collinear
assert msa.ordering == Ordering.FM
mag_struct_analyzer = CollinearMagneticStructureAnalyzer(self.Fe)
assert mag_struct_analyzer.is_magnetic
assert mag_struct_analyzer.is_collinear
assert mag_struct_analyzer.ordering == Ordering.FM

msa = CollinearMagneticStructureAnalyzer(
mag_struct_analyzer = CollinearMagneticStructureAnalyzer(
self.NiO,
make_primitive=False,
overwrite_magmom_mode="replace_all_if_undefined",
)
assert msa.number_of_magnetic_sites == 4
assert msa.number_of_unique_magnetic_sites() == 1
assert msa.types_of_magnetic_species == (Element.Ni,)
assert msa.get_exchange_group_info() == ("Fm-3m", 225)
assert mag_struct_analyzer.number_of_magnetic_sites == 4
assert mag_struct_analyzer.number_of_unique_magnetic_sites() == 1
assert mag_struct_analyzer.types_of_magnetic_species == (Element.Ni,)
assert mag_struct_analyzer.get_exchange_group_info() == ("Fm-3m", 225)

# https://github.com/materialsproject/pymatgen/pull/3574
mag_struct_analyzer = CollinearMagneticStructureAnalyzer(self.CuO_expt)
assert mag_struct_analyzer.ordering == Ordering.AFM

def test_str(self):
msa = CollinearMagneticStructureAnalyzer(self.NiO_AFM_001)
Expand Down
Binary file added tests/files/magnetic.example.CuO.mcif.gz
Binary file not shown.
Loading