From adafbb3c02fc0a733bf8b821bcb7430d27688136 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 17:26:02 -1000 Subject: [PATCH 01/18] Fix ASE Atoms object --- pymatgen/io/ase.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/pymatgen/io/ase.py b/pymatgen/io/ase.py index d91bda5af3c..bf883d14a3f 100644 --- a/pymatgen/io/ase.py +++ b/pymatgen/io/ase.py @@ -8,9 +8,8 @@ import warnings from typing import TYPE_CHECKING - +from monty.dev import requires import numpy as np -from monty.json import jsanitize from pymatgen.core.structure import Molecule, Structure @@ -23,6 +22,7 @@ from ase import Atoms from ase.calculators.singlepoint import SinglePointDFTCalculator from ase.constraints import FixAtoms + from ase.spacegroup import Spacegroup ase_loaded = True except ImportError: @@ -38,6 +38,7 @@ # NOTE: If making notable changes to this class, please ping @arosen93 on GitHub. # There are some subtleties in here, particularly related to spins/charges. +@requires(ase_loaded) class AseAtomsAdaptor: """Adaptor serves as a bridge between ASE Atoms and pymatgen objects.""" @@ -161,8 +162,12 @@ def get_atoms(structure: SiteCollection, **kwargs) -> Atoms: # Atoms.info <---> Structure.properties # Atoms.calc <---> Structure.calc - if structure.properties: - atoms.info = structure.properties + atoms.info = structure.properties + + # Regenerate Spacegroup object + if isinstance(atoms.info.get("spacegroup"),dict): + atoms.info["spacegroup"] = Spacegroup(atoms.info["spacegroup"]["number"],setting=atoms.info["spacegroup"].get("setting",1)) + if calc := getattr(structure, "calc", None): atoms.calc = calc @@ -218,7 +223,10 @@ def get_structure(atoms: Atoms, cls: type[Structure] = Structure, **cls_kwargs) sel_dyn = None # Atoms.info <---> Structure.properties - properties = jsanitize(getattr(atoms, "info", {})) + # But first make sure `spacegroup` is JSON serializable + if atoms.info.get("spacegroup"): + atoms.info["spacegroup"] = atoms.info["spacegroup"].todict() + properties = getattr(atoms, "info", {}) # Return a Molecule object if that was specifically requested; # otherwise return a Structure object as expected From 989c4718f5536b66b561ec4a2d2ceb6fe1d316f0 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 17:30:15 -1000 Subject: [PATCH 02/18] patch --- pymatgen/io/ase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pymatgen/io/ase.py b/pymatgen/io/ase.py index d5c60bbfbd8..408400a1e9c 100644 --- a/pymatgen/io/ase.py +++ b/pymatgen/io/ase.py @@ -38,7 +38,7 @@ # NOTE: If making notable changes to this class, please ping @Andrew-S-Rosen on GitHub. # There are some subtleties in here, particularly related to spins/charges. -@requires(ase_loaded) +@requires(ase_loaded, "ASE needs to be installed.") class AseAtomsAdaptor: """Adaptor serves as a bridge between ASE Atoms and pymatgen objects.""" From dda19f3c81f4c5e6666a548e1f706304bf33aa16 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 28 Sep 2023 03:30:20 +0000 Subject: [PATCH 03/18] pre-commit auto-fixes --- pymatgen/io/ase.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pymatgen/io/ase.py b/pymatgen/io/ase.py index 408400a1e9c..06a00d63ba5 100644 --- a/pymatgen/io/ase.py +++ b/pymatgen/io/ase.py @@ -8,8 +8,9 @@ import warnings from typing import TYPE_CHECKING -from monty.dev import requires + import numpy as np +from monty.dev import requires from pymatgen.core.structure import Molecule, Structure @@ -163,11 +164,13 @@ def get_atoms(structure: SiteCollection, **kwargs) -> Atoms: # Atoms.info <---> Structure.properties if properties := getattr(structure, "properties"): # noqa: B009 atoms.info = properties - + # Regenerate Spacegroup object if isinstance(atoms.info.get("spacegroup"), dict): - atoms.info["spacegroup"] = Spacegroup(atoms.info["spacegroup"]["number"], setting=atoms.info["spacegroup"].get("setting", 1)) - + atoms.info["spacegroup"] = Spacegroup( + atoms.info["spacegroup"]["number"], setting=atoms.info["spacegroup"].get("setting", 1) + ) + # Atoms.calc <---> Structure.calc if calc := getattr(structure, "calc", None): atoms.calc = calc From bcf62153ae9f03a93752cb7528d50d26e62f9128 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 17:34:30 -1000 Subject: [PATCH 04/18] add test --- tests/io/test_ase.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index 4ba7b54f12b..bd226de288f 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -228,12 +228,13 @@ def test_get_molecule(self): assert molecule.charge == 2 assert molecule.spin_multiplicity == 3 - def test_back_forth(self): + @pytest.mark.parametrize("structure_file", ["OUTCAR", "V2O3.cif"], indirect=True) + def test_back_forth(self, structure_file): from ase.constraints import FixAtoms from ase.io import read # Atoms --> Structure --> Atoms --> Structure - atoms = read(TEST_FILES_DIR / "OUTCAR") + atoms = read(TEST_FILES_DIR / structure_file) atoms.info = {"test": "hi"} atoms.set_tags([1] * len(atoms)) atoms.set_constraint(FixAtoms(mask=[True] * len(atoms))) @@ -295,3 +296,5 @@ def test_back_forth(self): # test document can be jsanitized and decoded d = jsanitize(molecule, strict=True, enum_values=True) MontyDecoder().process_decoded(d) + + def test_back_forth_v2(self): From cd0d1275e1b8ca174a7a66f83ae2d5b2bf6a71ae Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 28 Sep 2023 03:35:48 +0000 Subject: [PATCH 05/18] pre-commit auto-fixes --- tests/io/test_ase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index bd226de288f..f5b6cf98d6e 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -296,5 +296,5 @@ def test_back_forth(self, structure_file): # test document can be jsanitized and decoded d = jsanitize(molecule, strict=True, enum_values=True) MontyDecoder().process_decoded(d) - + def test_back_forth_v2(self): From ba7fec2fa32a3588b55a729ea0315ef9fbfb7542 Mon Sep 17 00:00:00 2001 From: "Andrew S. Rosen" Date: Wed, 27 Sep 2023 17:37:35 -1000 Subject: [PATCH 06/18] Update test_ase.py --- tests/io/test_ase.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index f5b6cf98d6e..43b185a258a 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -296,5 +296,3 @@ def test_back_forth(self, structure_file): # test document can be jsanitized and decoded d = jsanitize(molecule, strict=True, enum_values=True) MontyDecoder().process_decoded(d) - - def test_back_forth_v2(self): From 599360d1801995143cf3503d395ce2040a4c1db1 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 17:41:48 -1000 Subject: [PATCH 07/18] patch --- pymatgen/io/ase.py | 5 ----- tests/io/test_ase.py | 2 -- 2 files changed, 7 deletions(-) diff --git a/pymatgen/io/ase.py b/pymatgen/io/ase.py index 06a00d63ba5..382fd0b195a 100644 --- a/pymatgen/io/ase.py +++ b/pymatgen/io/ase.py @@ -55,11 +55,6 @@ def get_atoms(structure: SiteCollection, **kwargs) -> Atoms: Returns: Atoms: ASE Atoms object """ - if not ase_loaded: - raise ImportError( - "AseAtomsAdaptor requires the ASE package.\n" - "Use `pip install ase` or `conda install ase -c conda-forge`" - ) if not structure.is_ordered: raise ValueError("ASE Atoms only supports ordered structures") diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index bd226de288f..43b185a258a 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -296,5 +296,3 @@ def test_back_forth(self, structure_file): # test document can be jsanitized and decoded d = jsanitize(molecule, strict=True, enum_values=True) MontyDecoder().process_decoded(d) - - def test_back_forth_v2(self): From cc622afc454c81a5d6221137bad3ced2a8569890 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 17:51:31 -1000 Subject: [PATCH 08/18] fix test --- tests/io/test_ase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index 43b185a258a..6d049fda477 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -228,7 +228,7 @@ def test_get_molecule(self): assert molecule.charge == 2 assert molecule.spin_multiplicity == 3 - @pytest.mark.parametrize("structure_file", ["OUTCAR", "V2O3.cif"], indirect=True) + @pytest.mark.parametrize("structure_file", ["OUTCAR", "V2O3.cif"]) def test_back_forth(self, structure_file): from ase.constraints import FixAtoms from ase.io import read From 322010b073e2f45cc585a2e443db4d73285942b7 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 18:17:33 -1000 Subject: [PATCH 09/18] Clean up test suite --- pymatgen/io/ase.py | 31 +++++++++++++------------------ tests/io/test_ase.py | 38 ++++++++++++++++++-------------------- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/pymatgen/io/ase.py b/pymatgen/io/ase.py index 382fd0b195a..996bd74f945 100644 --- a/pymatgen/io/ase.py +++ b/pymatgen/io/ase.py @@ -8,6 +8,7 @@ import warnings from typing import TYPE_CHECKING +from collections.abc import Iterable import numpy as np from monty.dev import requires @@ -109,20 +110,6 @@ def get_atoms(structure: SiteCollection, **kwargs) -> Atoms: atoms.charge = structure.charge atoms.spin_multiplicity = structure.spin_multiplicity - # Set the Atoms final magnetic moments and charges if present. - # This uses the SinglePointDFTCalculator as the dummy calculator - # to store results. - charges = structure.site_properties.get("final_charge") - magmoms = structure.site_properties.get("final_magmom") - if magmoms or charges: - if magmoms and charges: - calc = SinglePointDFTCalculator(atoms, magmoms=magmoms, charges=charges) - elif magmoms: - calc = SinglePointDFTCalculator(atoms, magmoms=magmoms) - else: - calc = SinglePointDFTCalculator(atoms, charges=charges) - atoms.calc = calc - # Get the oxidation states from the structure oxi_states: list[float | None] = [getattr(site.specie, "oxi_state", None) for site in structure] @@ -133,8 +120,7 @@ def get_atoms(structure: SiteCollection, **kwargs) -> Atoms: fix_atoms = [] for site in structure: selective_dynamics: ArrayLike = site.properties.get("selective_dynamics") # type: ignore[assignment] - if not (np.all(selective_dynamics) or not np.any(selective_dynamics)): - # should be [True, True, True] or [False, False, False] + if isinstance(selective_dynamics,Iterable) and True in selective_dynamics and False in selective_dynamics: raise ValueError( "ASE FixAtoms constraint does not support selective dynamics in only some dimensions. " f"Remove the {selective_dynamics=} and try again if you do not need them." @@ -160,7 +146,7 @@ def get_atoms(structure: SiteCollection, **kwargs) -> Atoms: if properties := getattr(structure, "properties"): # noqa: B009 atoms.info = properties - # Regenerate Spacegroup object + # Regenerate Spacegroup object from `.todict()` representation if isinstance(atoms.info.get("spacegroup"), dict): atoms.info["spacegroup"] = Spacegroup( atoms.info["spacegroup"]["number"], setting=atoms.info["spacegroup"].get("setting", 1) @@ -169,6 +155,15 @@ def get_atoms(structure: SiteCollection, **kwargs) -> Atoms: # Atoms.calc <---> Structure.calc if calc := getattr(structure, "calc", None): atoms.calc = calc + else: + # Set the Atoms final magnetic moments and charges if present. + # This uses the SinglePointDFTCalculator as the dummy calculator + # to store results. + charges = structure.site_properties.get("final_charge") + magmoms = structure.site_properties.get("final_magmom") + if charges or magmoms: + calc = SinglePointDFTCalculator(atoms, magmoms=magmoms,charges=charges) + atoms.calc = calc return atoms @@ -216,7 +211,7 @@ def get_structure(atoms: Atoms, cls: type[Structure] = Structure, **cls_kwargs) else: unsupported_constraint_type = True if unsupported_constraint_type: - warnings.warn("Only FixAtoms is supported by Pymatgen. Other constraints will not be set.") + warnings.warn("Only FixAtoms is supported by Pymatgen. Other constraints will not be set.", UserWarning) sel_dyn = [[False] * 3 if atom.index in constraint_indices else [True] * 3 for atom in atoms] else: sel_dyn = None diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index 6d049fda477..d4cba89d3ad 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -12,6 +12,7 @@ from pymatgen.util.testing import TEST_FILES_DIR poscar = Poscar.from_file(TEST_FILES_DIR / "POSCAR") +ase = pytest.importorskip("ase") class TestAseAtomsAdaptor: @@ -171,7 +172,11 @@ def test_get_structure_mag(self): assert "magmom" not in structure.site_properties assert "initial_magmoms" not in structure.site_properties - def test_get_structure_dyn(self): + @pytest.mark.parametrize("select_dyn", [[True, True, True], + [False, False, False], + np.array([True, True, True]), + np.array([False, False, False])]) + def test_get_structure_dyn(self, select_dyn): from ase.constraints import FixAtoms from ase.io import read @@ -180,25 +185,18 @@ def test_get_structure_dyn(self): structure = aio.AseAtomsAdaptor.get_structure(atoms) assert structure.site_properties["selective_dynamics"][-1][0] is False - # https://github.com/materialsproject/pymatgen/issues/3011 - for select_dyn in ( - [True, True, True], - [False, False, False], - np.array([True, True, True]), - np.array([False, False, False]), - ): - structure = Structure( - lattice=Lattice.cubic(5), - species=("Fe", "O"), - coords=((0, 0, 0), (0.5, 0.5, 0.5)), - site_properties={"selective_dynamics": select_dyn}, - ) - structure.sites[0].selective_dynamics = select_dyn - - # mostly testing that this call doesn't raise - ase_atoms = AseAtomsAdaptor.get_atoms(structure) - - assert len(ase_atoms) == len(structure) + structure = Structure( + lattice=Lattice.cubic(5), + species=("Fe", "O"), + coords=((0, 0, 0), (0.5, 0.5, 0.5)), + site_properties={"selective_dynamics": select_dyn}, + ) + structure.sites[0].selective_dynamics = select_dyn + + # mostly testing that this call doesn't raise + ase_atoms = AseAtomsAdaptor.get_atoms(structure) + + assert len(ase_atoms) == len(structure) def test_get_molecule(self): from ase.io import read From 09c894cd11ba8cb6775fc53f1a6fe3df6521b49a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 28 Sep 2023 04:18:40 +0000 Subject: [PATCH 10/18] pre-commit auto-fixes --- pymatgen/io/ase.py | 10 +++++++--- tests/io/test_ase.py | 8 ++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pymatgen/io/ase.py b/pymatgen/io/ase.py index 996bd74f945..8d61211e8c8 100644 --- a/pymatgen/io/ase.py +++ b/pymatgen/io/ase.py @@ -7,8 +7,8 @@ from __future__ import annotations import warnings -from typing import TYPE_CHECKING from collections.abc import Iterable +from typing import TYPE_CHECKING import numpy as np from monty.dev import requires @@ -120,7 +120,11 @@ def get_atoms(structure: SiteCollection, **kwargs) -> Atoms: fix_atoms = [] for site in structure: selective_dynamics: ArrayLike = site.properties.get("selective_dynamics") # type: ignore[assignment] - if isinstance(selective_dynamics,Iterable) and True in selective_dynamics and False in selective_dynamics: + if ( + isinstance(selective_dynamics, Iterable) + and True in selective_dynamics + and False in selective_dynamics + ): raise ValueError( "ASE FixAtoms constraint does not support selective dynamics in only some dimensions. " f"Remove the {selective_dynamics=} and try again if you do not need them." @@ -162,7 +166,7 @@ def get_atoms(structure: SiteCollection, **kwargs) -> Atoms: charges = structure.site_properties.get("final_charge") magmoms = structure.site_properties.get("final_magmom") if charges or magmoms: - calc = SinglePointDFTCalculator(atoms, magmoms=magmoms,charges=charges) + calc = SinglePointDFTCalculator(atoms, magmoms=magmoms, charges=charges) atoms.calc = calc return atoms diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index d4cba89d3ad..79cc69d0707 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -172,10 +172,10 @@ def test_get_structure_mag(self): assert "magmom" not in structure.site_properties assert "initial_magmoms" not in structure.site_properties - @pytest.mark.parametrize("select_dyn", [[True, True, True], - [False, False, False], - np.array([True, True, True]), - np.array([False, False, False])]) + @pytest.mark.parametrize( + "select_dyn", + [[True, True, True], [False, False, False], np.array([True, True, True]), np.array([False, False, False])], + ) def test_get_structure_dyn(self, select_dyn): from ase.constraints import FixAtoms from ase.io import read From 6300b0bf9af858af6790bcb247b548792004b278 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 18:20:37 -1000 Subject: [PATCH 11/18] Clean up test suite --- tests/io/test_ase.py | 554 +++++++++++++++++++++---------------------- 1 file changed, 274 insertions(+), 280 deletions(-) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index d4cba89d3ad..870a508c9b0 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -14,283 +14,277 @@ poscar = Poscar.from_file(TEST_FILES_DIR / "POSCAR") ase = pytest.importorskip("ase") - -class TestAseAtomsAdaptor: - def test_get_atoms_from_structure(self): - structure = poscar.structure - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - ase_composition = Composition(atoms.get_chemical_formula()) - assert ase_composition == structure.composition - assert atoms.cell is not None - assert atoms.cell.any() - assert atoms.get_pbc() is not None - assert atoms.get_pbc().all() - assert atoms.get_chemical_symbols() == [s.species_string for s in structure] - assert not atoms.has("initial_magmoms") - assert not atoms.has("initial_charges") - assert atoms.calc is None - - structure = poscar.structure - prop = [3.14] * len(structure) - structure.add_site_property("prop", prop) - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - assert atoms.get_array("prop").tolist() == prop - - def test_get_atoms_from_structure_mags(self): - structure = poscar.structure - mags = [1.0] * len(structure) - structure.add_site_property("final_magmom", mags) - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - assert not atoms.has("initial_magmoms") - assert atoms.get_magnetic_moments().tolist() == mags - - structure = poscar.structure - initial_mags = [0.5] * len(structure) - structure.add_site_property("magmom", initial_mags) - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - assert atoms.get_initial_magnetic_moments().tolist() == initial_mags - - structure = poscar.structure - mags = [1.0] * len(structure) - structure.add_site_property("final_magmom", mags) - initial_mags = [2.0] * len(structure) - structure.add_site_property("magmom", initial_mags) - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - assert atoms.get_initial_magnetic_moments().tolist(), initial_mags - assert atoms.get_magnetic_moments().tolist(), mags - - def test_get_atoms_from_structure_charge(self): - structure = poscar.structure - charges = [1.0] * len(structure) - structure.add_site_property("final_charge", charges) - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - assert not atoms.has("initial_charges") - assert atoms.get_charges().tolist() == charges - - structure = poscar.structure - charges = [0.5] * len(structure) - structure.add_site_property("charge", charges) - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - assert atoms.get_initial_charges().tolist() == charges - - structure = poscar.structure - charges = [1.0] * len(structure) - structure.add_site_property("final_charge", charges) - initial_charges = [2.0] * len(structure) - structure.add_site_property("charge", initial_charges) - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - assert atoms.get_initial_charges().tolist(), initial_charges - assert atoms.get_charges().tolist(), charges - - def test_get_atoms_from_structure_oxi_states(self): - structure = poscar.structure - oxi_states = [1.0] * len(structure) - structure.add_oxidation_state_by_site(oxi_states) - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - assert atoms.get_array("oxi_states").tolist() == oxi_states - - def test_get_atoms_from_structure_dyn(self): - structure = poscar.structure - structure.add_site_property("selective_dynamics", [[False] * 3] * len(structure)) - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - assert atoms.constraints[0].get_indices().tolist() == [atom.index for atom in atoms] - - def test_get_atoms_from_molecule(self): - m = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") - atoms = aio.AseAtomsAdaptor.get_atoms(m) - ase_composition = Composition(atoms.get_chemical_formula()) - assert ase_composition == m.composition - assert atoms.cell is None or not atoms.cell.any() - assert atoms.get_pbc() is None or not atoms.get_pbc().any() - assert atoms.get_chemical_symbols() == [s.species_string for s in m] - assert not atoms.has("initial_magmoms") - - def test_get_atoms_from_molecule_mags(self): - molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") - atoms = aio.AseAtomsAdaptor.get_atoms(molecule) - mags = [1.0] * len(molecule) - molecule.add_site_property("final_magmom", mags) - atoms = aio.AseAtomsAdaptor.get_atoms(molecule) - assert not atoms.has("initial_magmoms") - assert atoms.get_magnetic_moments().tolist() == mags - - molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") - atoms = aio.AseAtomsAdaptor.get_atoms(molecule) - initial_mags = [0.5] * len(molecule) - molecule.add_site_property("magmom", initial_mags) - atoms = aio.AseAtomsAdaptor.get_atoms(molecule) - assert atoms.get_initial_magnetic_moments().tolist() == initial_mags - - molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") - molecule.set_charge_and_spin(-2, spin_multiplicity=3) - atoms = aio.AseAtomsAdaptor.get_atoms(molecule) - assert atoms.calc is None - assert atoms.get_initial_magnetic_moments().tolist() == [0] * len(molecule) - assert atoms.charge == -2 - assert atoms.spin_multiplicity == 3 - - def test_get_atoms_from_molecule_dyn(self): - molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") - molecule.add_site_property("selective_dynamics", [[False] * 3] * len(molecule)) - atoms = aio.AseAtomsAdaptor.get_atoms(molecule) - assert atoms.constraints[0].get_indices().tolist() == [atom.index for atom in atoms] - - def test_get_structure(self): - from ase.io import read - - atoms = read(TEST_FILES_DIR / "POSCAR") - struct = aio.AseAtomsAdaptor.get_structure(atoms) - assert struct.formula == "Fe4 P4 O16" - assert [s.species_string for s in struct] == atoms.get_chemical_symbols() - - atoms = read(TEST_FILES_DIR / "POSCAR") - prop = np.array([3.14] * len(atoms)) - atoms.set_array("prop", prop) - struct = aio.AseAtomsAdaptor.get_structure(atoms) - assert struct.site_properties["prop"] == prop.tolist() - - atoms = read(TEST_FILES_DIR / "POSCAR_overlap") - struct = aio.AseAtomsAdaptor.get_structure(atoms) - assert [s.species_string for s in struct] == atoms.get_chemical_symbols() - with pytest.raises(StructureError, match="Structure contains sites that are less than 0.01 Angstrom apart"): - struct = aio.AseAtomsAdaptor.get_structure(atoms, validate_proximity=True) - - def test_get_structure_mag(self): - from ase.io import read - - atoms = read(TEST_FILES_DIR / "POSCAR") - mags = [1.0] * len(atoms) - atoms.set_initial_magnetic_moments(mags) - structure = aio.AseAtomsAdaptor.get_structure(atoms) - assert structure.site_properties["magmom"] == mags - assert "final_magmom" not in structure.site_properties - assert "initial_magmoms" not in structure.site_properties - - atoms = read(TEST_FILES_DIR / "OUTCAR") - structure = aio.AseAtomsAdaptor.get_structure(atoms) - assert structure.site_properties["final_magmom"] == atoms.get_magnetic_moments().tolist() - assert "magmom" not in structure.site_properties - assert "initial_magmoms" not in structure.site_properties - - @pytest.mark.parametrize("select_dyn", [[True, True, True], - [False, False, False], - np.array([True, True, True]), - np.array([False, False, False])]) - def test_get_structure_dyn(self, select_dyn): - from ase.constraints import FixAtoms - from ase.io import read - - atoms = read(TEST_FILES_DIR / "POSCAR") - atoms.set_constraint(FixAtoms(mask=[True] * len(atoms))) - structure = aio.AseAtomsAdaptor.get_structure(atoms) - assert structure.site_properties["selective_dynamics"][-1][0] is False - - structure = Structure( - lattice=Lattice.cubic(5), - species=("Fe", "O"), - coords=((0, 0, 0), (0.5, 0.5, 0.5)), - site_properties={"selective_dynamics": select_dyn}, - ) - structure.sites[0].selective_dynamics = select_dyn - - # mostly testing that this call doesn't raise - ase_atoms = AseAtomsAdaptor.get_atoms(structure) - - assert len(ase_atoms) == len(structure) - - def test_get_molecule(self): - from ase.io import read - - atoms = read(TEST_FILES_DIR / "acetylene.xyz") - molecule = aio.AseAtomsAdaptor.get_molecule(atoms) - assert molecule.formula == "H2 C2" - assert [s.species_string for s in molecule] == atoms.get_chemical_symbols() - assert molecule.charge == 0 - assert molecule.spin_multiplicity == 1 - - atoms = read(TEST_FILES_DIR / "acetylene.xyz") - initial_charges = [2.0] * len(atoms) - initial_mags = [1.0] * len(atoms) - atoms.set_initial_charges(initial_charges) - atoms.set_initial_magnetic_moments(initial_mags) - molecule = aio.AseAtomsAdaptor.get_molecule(atoms) - assert molecule.charge == np.sum(initial_charges) - assert molecule.spin_multiplicity == np.sum(initial_mags) + 1 - assert molecule.site_properties.get("charge") == initial_charges - assert molecule.site_properties.get("magmom") == initial_mags - - atoms = read(TEST_FILES_DIR / "acetylene.xyz") - atoms.spin_multiplicity = 3 - atoms.charge = 2 - molecule = aio.AseAtomsAdaptor.get_molecule(atoms) - assert molecule.charge == 2 - assert molecule.spin_multiplicity == 3 - - @pytest.mark.parametrize("structure_file", ["OUTCAR", "V2O3.cif"]) - def test_back_forth(self, structure_file): - from ase.constraints import FixAtoms - from ase.io import read - - # Atoms --> Structure --> Atoms --> Structure - atoms = read(TEST_FILES_DIR / structure_file) - atoms.info = {"test": "hi"} - atoms.set_tags([1] * len(atoms)) - atoms.set_constraint(FixAtoms(mask=[True] * len(atoms))) - atoms.set_initial_charges([1.0] * len(atoms)) - atoms.set_initial_magnetic_moments([2.0] * len(atoms)) - atoms.set_array("prop", np.array([3.0] * len(atoms))) - structure = aio.AseAtomsAdaptor.get_structure(atoms) - atoms_back = aio.AseAtomsAdaptor.get_atoms(structure) - structure_back = aio.AseAtomsAdaptor.get_structure(atoms_back) - assert structure_back == structure - for k, v in atoms.todict().items(): - assert str(atoms_back.todict()[k]) == str(v) - - # Structure --> Atoms --> Structure --> Atoms - structure = Structure.from_file(TEST_FILES_DIR / "POSCAR") - structure.add_site_property("final_magmom", [1.0] * len(structure)) - structure.add_site_property("magmom", [2.0] * len(structure)) - structure.add_site_property("final_charge", [3.0] * len(structure)) - structure.add_site_property("charge", [4.0] * len(structure)) - structure.add_site_property("prop", [5.0] * len(structure)) - structure.properties = {"test": "hi"} - atoms = aio.AseAtomsAdaptor.get_atoms(structure) - structure_back = aio.AseAtomsAdaptor.get_structure(atoms) - atoms_back = aio.AseAtomsAdaptor.get_atoms(structure_back) - assert structure_back == structure - for k, v in atoms.todict().items(): - assert str(atoms_back.todict()[k]) == str(v) - - # test document can be jsanitized and decoded - d = jsanitize(structure, strict=True, enum_values=True) - MontyDecoder().process_decoded(d) - - # Atoms --> Molecule --> Atoms --> Molecule - atoms = read(TEST_FILES_DIR / "acetylene.xyz") - atoms.info = {"test": "hi"} - atoms.set_constraint(FixAtoms(mask=[True] * len(atoms))) - atoms.set_initial_charges([1.0] * len(atoms)) - atoms.set_initial_magnetic_moments([2.0] * len(atoms)) - atoms.set_array("prop", np.array([3.0] * len(atoms))) - atoms.set_tags([1] * len(atoms)) - molecule = aio.AseAtomsAdaptor.get_molecule(atoms) - atoms_back = aio.AseAtomsAdaptor.get_atoms(molecule) - molecule_back = aio.AseAtomsAdaptor.get_molecule(atoms_back) - for k, v in atoms.todict().items(): - assert str(atoms_back.todict()[k]) == str(v) - assert molecule_back == molecule - - # Molecule --> Atoms --> Molecule --> Atoms - molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") - molecule.set_charge_and_spin(-2, spin_multiplicity=3) - molecule.properties = {"test": "hi"} - atoms = aio.AseAtomsAdaptor.get_atoms(molecule) - molecule_back = aio.AseAtomsAdaptor.get_molecule(atoms) - atoms_back = aio.AseAtomsAdaptor.get_atoms(molecule_back) - for k, v in atoms.todict().items(): - assert str(atoms_back.todict()[k]) == str(v) - assert molecule_back == molecule - - # test document can be jsanitized and decoded - d = jsanitize(molecule, strict=True, enum_values=True) - MontyDecoder().process_decoded(d) +from ase.constraints import FixAtoms +from ase.io import read + +def test_get_atoms_from_structure(): + structure = poscar.structure + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + ase_composition = Composition(atoms.get_chemical_formula()) + assert ase_composition == structure.composition + assert atoms.cell is not None + assert atoms.cell.any() + assert atoms.get_pbc() is not None + assert atoms.get_pbc().all() + assert atoms.get_chemical_symbols() == [s.species_string for s in structure] + assert not atoms.has("initial_magmoms") + assert not atoms.has("initial_charges") + assert atoms.calc is None + + structure = poscar.structure + prop = [3.14] * len(structure) + structure.add_site_property("prop", prop) + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + assert atoms.get_array("prop").tolist() == prop + +def test_get_atoms_from_structure_mags(): + structure = poscar.structure + mags = [1.0] * len(structure) + structure.add_site_property("final_magmom", mags) + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + assert not atoms.has("initial_magmoms") + assert atoms.get_magnetic_moments().tolist() == mags + + structure = poscar.structure + initial_mags = [0.5] * len(structure) + structure.add_site_property("magmom", initial_mags) + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + assert atoms.get_initial_magnetic_moments().tolist() == initial_mags + + structure = poscar.structure + mags = [1.0] * len(structure) + structure.add_site_property("final_magmom", mags) + initial_mags = [2.0] * len(structure) + structure.add_site_property("magmom", initial_mags) + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + assert atoms.get_initial_magnetic_moments().tolist(), initial_mags + assert atoms.get_magnetic_moments().tolist(), mags + +def test_get_atoms_from_structure_charge(): + structure = poscar.structure + charges = [1.0] * len(structure) + structure.add_site_property("final_charge", charges) + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + assert not atoms.has("initial_charges") + assert atoms.get_charges().tolist() == charges + + structure = poscar.structure + charges = [0.5] * len(structure) + structure.add_site_property("charge", charges) + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + assert atoms.get_initial_charges().tolist() == charges + + structure = poscar.structure + charges = [1.0] * len(structure) + structure.add_site_property("final_charge", charges) + initial_charges = [2.0] * len(structure) + structure.add_site_property("charge", initial_charges) + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + assert atoms.get_initial_charges().tolist(), initial_charges + assert atoms.get_charges().tolist(), charges + +def test_get_atoms_from_structure_oxi_states(): + structure = poscar.structure + oxi_states = [1.0] * len(structure) + structure.add_oxidation_state_by_site(oxi_states) + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + assert atoms.get_array("oxi_states").tolist() == oxi_states + +def test_get_atoms_from_structure_dyn(): + structure = poscar.structure + structure.add_site_property("selective_dynamics", [[False] * 3] * len(structure)) + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + assert atoms.constraints[0].get_indices().tolist() == [atom.index for atom in atoms] + +def test_get_atoms_from_molecule(): + m = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") + atoms = aio.AseAtomsAdaptor.get_atoms(m) + ase_composition = Composition(atoms.get_chemical_formula()) + assert ase_composition == m.composition + assert atoms.cell is None or not atoms.cell.any() + assert atoms.get_pbc() is None or not atoms.get_pbc().any() + assert atoms.get_chemical_symbols() == [s.species_string for s in m] + assert not atoms.has("initial_magmoms") + +def test_get_atoms_from_molecule_mags(): + molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") + atoms = aio.AseAtomsAdaptor.get_atoms(molecule) + mags = [1.0] * len(molecule) + molecule.add_site_property("final_magmom", mags) + atoms = aio.AseAtomsAdaptor.get_atoms(molecule) + assert not atoms.has("initial_magmoms") + assert atoms.get_magnetic_moments().tolist() == mags + + molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") + atoms = aio.AseAtomsAdaptor.get_atoms(molecule) + initial_mags = [0.5] * len(molecule) + molecule.add_site_property("magmom", initial_mags) + atoms = aio.AseAtomsAdaptor.get_atoms(molecule) + assert atoms.get_initial_magnetic_moments().tolist() == initial_mags + + molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") + molecule.set_charge_and_spin(-2, spin_multiplicity=3) + atoms = aio.AseAtomsAdaptor.get_atoms(molecule) + assert atoms.calc is None + assert atoms.get_initial_magnetic_moments().tolist() == [0] * len(molecule) + assert atoms.charge == -2 + assert atoms.spin_multiplicity == 3 + +def test_get_atoms_from_molecule_dyn(): + molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") + molecule.add_site_property("selective_dynamics", [[False] * 3] * len(molecule)) + atoms = aio.AseAtomsAdaptor.get_atoms(molecule) + assert atoms.constraints[0].get_indices().tolist() == [atom.index for atom in atoms] + +def test_get_structure(): + + atoms = read(TEST_FILES_DIR / "POSCAR") + struct = aio.AseAtomsAdaptor.get_structure(atoms) + assert struct.formula == "Fe4 P4 O16" + assert [s.species_string for s in struct] == atoms.get_chemical_symbols() + + atoms = read(TEST_FILES_DIR / "POSCAR") + prop = np.array([3.14] * len(atoms)) + atoms.set_array("prop", prop) + struct = aio.AseAtomsAdaptor.get_structure(atoms) + assert struct.site_properties["prop"] == prop.tolist() + + atoms = read(TEST_FILES_DIR / "POSCAR_overlap") + struct = aio.AseAtomsAdaptor.get_structure(atoms) + assert [s.species_string for s in struct] == atoms.get_chemical_symbols() + with pytest.raises(StructureError, match="Structure contains sites that are less than 0.01 Angstrom apart"): + struct = aio.AseAtomsAdaptor.get_structure(atoms, validate_proximity=True) + +def test_get_structure_mag(): + + atoms = read(TEST_FILES_DIR / "POSCAR") + mags = [1.0] * len(atoms) + atoms.set_initial_magnetic_moments(mags) + structure = aio.AseAtomsAdaptor.get_structure(atoms) + assert structure.site_properties["magmom"] == mags + assert "final_magmom" not in structure.site_properties + assert "initial_magmoms" not in structure.site_properties + + atoms = read(TEST_FILES_DIR / "OUTCAR") + structure = aio.AseAtomsAdaptor.get_structure(atoms) + assert structure.site_properties["final_magmom"] == atoms.get_magnetic_moments().tolist() + assert "magmom" not in structure.site_properties + assert "initial_magmoms" not in structure.site_properties + +@pytest.mark.parametrize("select_dyn", [[True, True, True], + [False, False, False], + np.array([True, True, True]), + np.array([False, False, False])]) +def test_get_structure_dyn(select_dyn): + + atoms = read(TEST_FILES_DIR / "POSCAR") + atoms.set_constraint(FixAtoms(mask=[True] * len(atoms))) + structure = aio.AseAtomsAdaptor.get_structure(atoms) + assert structure.site_properties["selective_dynamics"][-1][0] is False + + structure = Structure( + lattice=Lattice.cubic(5), + species=("Fe", "O"), + coords=((0, 0, 0), (0.5, 0.5, 0.5)), + site_properties={"selective_dynamics": select_dyn}, + ) + structure.sites[0].selective_dynamics = select_dyn + + # mostly testing that this call doesn't raise + ase_atoms = AseAtomsAdaptor.get_atoms(structure) + + assert len(ase_atoms) == len(structure) + +def test_get_molecule(): + + atoms = read(TEST_FILES_DIR / "acetylene.xyz") + molecule = aio.AseAtomsAdaptor.get_molecule(atoms) + assert molecule.formula == "H2 C2" + assert [s.species_string for s in molecule] == atoms.get_chemical_symbols() + assert molecule.charge == 0 + assert molecule.spin_multiplicity == 1 + + atoms = read(TEST_FILES_DIR / "acetylene.xyz") + initial_charges = [2.0] * len(atoms) + initial_mags = [1.0] * len(atoms) + atoms.set_initial_charges(initial_charges) + atoms.set_initial_magnetic_moments(initial_mags) + molecule = aio.AseAtomsAdaptor.get_molecule(atoms) + assert molecule.charge == np.sum(initial_charges) + assert molecule.spin_multiplicity == np.sum(initial_mags) + 1 + assert molecule.site_properties.get("charge") == initial_charges + assert molecule.site_properties.get("magmom") == initial_mags + + atoms = read(TEST_FILES_DIR / "acetylene.xyz") + atoms.spin_multiplicity = 3 + atoms.charge = 2 + molecule = aio.AseAtomsAdaptor.get_molecule(atoms) + assert molecule.charge == 2 + assert molecule.spin_multiplicity == 3 + +@pytest.mark.parametrize("structure_file", ["OUTCAR", "V2O3.cif"]) +def test_back_forth(structure_file): + + # Atoms --> Structure --> Atoms --> Structure + atoms = read(TEST_FILES_DIR / structure_file) + atoms.info = {"test": "hi"} + atoms.set_tags([1] * len(atoms)) + atoms.set_constraint(FixAtoms(mask=[True] * len(atoms))) + atoms.set_initial_charges([1.0] * len(atoms)) + atoms.set_initial_magnetic_moments([2.0] * len(atoms)) + atoms.set_array("prop", np.array([3.0] * len(atoms))) + structure = aio.AseAtomsAdaptor.get_structure(atoms) + atoms_back = aio.AseAtomsAdaptor.get_atoms(structure) + structure_back = aio.AseAtomsAdaptor.get_structure(atoms_back) + assert structure_back == structure + for k, v in atoms.todict().items(): + assert str(atoms_back.todict()[k]) == str(v) + + # Structure --> Atoms --> Structure --> Atoms + structure = Structure.from_file(TEST_FILES_DIR / "POSCAR") + structure.add_site_property("final_magmom", [1.0] * len(structure)) + structure.add_site_property("magmom", [2.0] * len(structure)) + structure.add_site_property("final_charge", [3.0] * len(structure)) + structure.add_site_property("charge", [4.0] * len(structure)) + structure.add_site_property("prop", [5.0] * len(structure)) + structure.properties = {"test": "hi"} + atoms = aio.AseAtomsAdaptor.get_atoms(structure) + structure_back = aio.AseAtomsAdaptor.get_structure(atoms) + atoms_back = aio.AseAtomsAdaptor.get_atoms(structure_back) + assert structure_back == structure + for k, v in atoms.todict().items(): + assert str(atoms_back.todict()[k]) == str(v) + + # test document can be jsanitized and decoded + d = jsanitize(structure, strict=True, enum_values=True) + MontyDecoder().process_decoded(d) + + # Atoms --> Molecule --> Atoms --> Molecule + atoms = read(TEST_FILES_DIR / "acetylene.xyz") + atoms.info = {"test": "hi"} + atoms.set_constraint(FixAtoms(mask=[True] * len(atoms))) + atoms.set_initial_charges([1.0] * len(atoms)) + atoms.set_initial_magnetic_moments([2.0] * len(atoms)) + atoms.set_array("prop", np.array([3.0] * len(atoms))) + atoms.set_tags([1] * len(atoms)) + molecule = aio.AseAtomsAdaptor.get_molecule(atoms) + atoms_back = aio.AseAtomsAdaptor.get_atoms(molecule) + molecule_back = aio.AseAtomsAdaptor.get_molecule(atoms_back) + for k, v in atoms.todict().items(): + assert str(atoms_back.todict()[k]) == str(v) + assert molecule_back == molecule + + # Molecule --> Atoms --> Molecule --> Atoms + molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") + molecule.set_charge_and_spin(-2, spin_multiplicity=3) + molecule.properties = {"test": "hi"} + atoms = aio.AseAtomsAdaptor.get_atoms(molecule) + molecule_back = aio.AseAtomsAdaptor.get_molecule(atoms) + atoms_back = aio.AseAtomsAdaptor.get_atoms(molecule_back) + for k, v in atoms.todict().items(): + assert str(atoms_back.todict()[k]) == str(v) + assert molecule_back == molecule + + # test document can be jsanitized and decoded + d = jsanitize(molecule, strict=True, enum_values=True) + MontyDecoder().process_decoded(d) From aed40774c39447769a5876689fffdc644720c268 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 18:27:59 -1000 Subject: [PATCH 12/18] patch --- pymatgen/io/ase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pymatgen/io/ase.py b/pymatgen/io/ase.py index 8d61211e8c8..9068cb3b76f 100644 --- a/pymatgen/io/ase.py +++ b/pymatgen/io/ase.py @@ -222,7 +222,7 @@ def get_structure(atoms: Atoms, cls: type[Structure] = Structure, **cls_kwargs) # Atoms.info <---> Structure.properties # But first make sure `spacegroup` is JSON serializable - if atoms.info.get("spacegroup"): + if atoms.info.get("spacegroup") and isinstance(atoms.info["spacegroup"], Spacegroup): atoms.info["spacegroup"] = atoms.info["spacegroup"].todict() properties = getattr(atoms, "info", {}) From 369c98dceb984fa9be7ad4ea6a857d4348467bd1 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 18:34:01 -1000 Subject: [PATCH 13/18] fix merge issue --- tests/io/test_ase.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index 34d53b8a8e5..257db1f8390 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -171,11 +171,11 @@ def test_get_structure_mag(): assert "magmom" not in structure.site_properties assert "initial_magmoms" not in structure.site_properties - @pytest.mark.parametrize( - "select_dyn", - [[True, True, True], [False, False, False], np.array([True, True, True]), np.array([False, False, False])], - ) - def test_get_structure_dyn(self, select_dyn): +@pytest.mark.parametrize( + "select_dyn", + [[True, True, True], [False, False, False], np.array([True, True, True]), np.array([False, False, False])], +) +def test_get_structure_dyn(select_dyn): atoms = read(TEST_FILES_DIR / "POSCAR") atoms.set_constraint(FixAtoms(mask=[True] * len(atoms))) From 10e21c3b08abd4af935f8e0f13175be1c222fa3a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 28 Sep 2023 04:35:12 +0000 Subject: [PATCH 14/18] pre-commit auto-fixes --- tests/io/test_ase.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index 257db1f8390..d5b9d8fdd0d 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -17,6 +17,7 @@ from ase.constraints import FixAtoms from ase.io import read + def test_get_atoms_from_structure(): structure = poscar.structure atoms = aio.AseAtomsAdaptor.get_atoms(structure) @@ -37,6 +38,7 @@ def test_get_atoms_from_structure(): atoms = aio.AseAtomsAdaptor.get_atoms(structure) assert atoms.get_array("prop").tolist() == prop + def test_get_atoms_from_structure_mags(): structure = poscar.structure mags = [1.0] * len(structure) @@ -60,6 +62,7 @@ def test_get_atoms_from_structure_mags(): assert atoms.get_initial_magnetic_moments().tolist(), initial_mags assert atoms.get_magnetic_moments().tolist(), mags + def test_get_atoms_from_structure_charge(): structure = poscar.structure charges = [1.0] * len(structure) @@ -83,6 +86,7 @@ def test_get_atoms_from_structure_charge(): assert atoms.get_initial_charges().tolist(), initial_charges assert atoms.get_charges().tolist(), charges + def test_get_atoms_from_structure_oxi_states(): structure = poscar.structure oxi_states = [1.0] * len(structure) @@ -90,12 +94,14 @@ def test_get_atoms_from_structure_oxi_states(): atoms = aio.AseAtomsAdaptor.get_atoms(structure) assert atoms.get_array("oxi_states").tolist() == oxi_states + def test_get_atoms_from_structure_dyn(): structure = poscar.structure structure.add_site_property("selective_dynamics", [[False] * 3] * len(structure)) atoms = aio.AseAtomsAdaptor.get_atoms(structure) assert atoms.constraints[0].get_indices().tolist() == [atom.index for atom in atoms] + def test_get_atoms_from_molecule(): m = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") atoms = aio.AseAtomsAdaptor.get_atoms(m) @@ -106,6 +112,7 @@ def test_get_atoms_from_molecule(): assert atoms.get_chemical_symbols() == [s.species_string for s in m] assert not atoms.has("initial_magmoms") + def test_get_atoms_from_molecule_mags(): molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") atoms = aio.AseAtomsAdaptor.get_atoms(molecule) @@ -130,14 +137,15 @@ def test_get_atoms_from_molecule_mags(): assert atoms.charge == -2 assert atoms.spin_multiplicity == 3 + def test_get_atoms_from_molecule_dyn(): molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") molecule.add_site_property("selective_dynamics", [[False] * 3] * len(molecule)) atoms = aio.AseAtomsAdaptor.get_atoms(molecule) assert atoms.constraints[0].get_indices().tolist() == [atom.index for atom in atoms] -def test_get_structure(): +def test_get_structure(): atoms = read(TEST_FILES_DIR / "POSCAR") struct = aio.AseAtomsAdaptor.get_structure(atoms) assert struct.formula == "Fe4 P4 O16" @@ -155,8 +163,8 @@ def test_get_structure(): with pytest.raises(StructureError, match="Structure contains sites that are less than 0.01 Angstrom apart"): struct = aio.AseAtomsAdaptor.get_structure(atoms, validate_proximity=True) -def test_get_structure_mag(): +def test_get_structure_mag(): atoms = read(TEST_FILES_DIR / "POSCAR") mags = [1.0] * len(atoms) atoms.set_initial_magnetic_moments(mags) @@ -171,12 +179,12 @@ def test_get_structure_mag(): assert "magmom" not in structure.site_properties assert "initial_magmoms" not in structure.site_properties + @pytest.mark.parametrize( "select_dyn", [[True, True, True], [False, False, False], np.array([True, True, True]), np.array([False, False, False])], ) def test_get_structure_dyn(select_dyn): - atoms = read(TEST_FILES_DIR / "POSCAR") atoms.set_constraint(FixAtoms(mask=[True] * len(atoms))) structure = aio.AseAtomsAdaptor.get_structure(atoms) @@ -195,8 +203,8 @@ def test_get_structure_dyn(select_dyn): assert len(ase_atoms) == len(structure) -def test_get_molecule(): +def test_get_molecule(): atoms = read(TEST_FILES_DIR / "acetylene.xyz") molecule = aio.AseAtomsAdaptor.get_molecule(atoms) assert molecule.formula == "H2 C2" @@ -222,9 +230,9 @@ def test_get_molecule(): assert molecule.charge == 2 assert molecule.spin_multiplicity == 3 + @pytest.mark.parametrize("structure_file", ["OUTCAR", "V2O3.cif"]) def test_back_forth(structure_file): - # Atoms --> Structure --> Atoms --> Structure atoms = read(TEST_FILES_DIR / structure_file) atoms.info = {"test": "hi"} From d18b6b787585c2c49888f0b162739cf297622d24 Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 18:37:28 -1000 Subject: [PATCH 15/18] fix ruff --- tests/io/test_ase.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index 257db1f8390..de920580592 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -14,8 +14,8 @@ poscar = Poscar.from_file(TEST_FILES_DIR / "POSCAR") ase = pytest.importorskip("ase") -from ase.constraints import FixAtoms -from ase.io import read +from ase.constraints import FixAtoms # noqa: E402 +from ase.io import read # noqa: E402 def test_get_atoms_from_structure(): structure = poscar.structure From e3a721becb893dd88f597752270657dbdee6e899 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 28 Sep 2023 04:39:27 +0000 Subject: [PATCH 16/18] pre-commit auto-fixes --- tests/io/test_ase.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index 4fddfa421e6..517d68f7419 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -14,8 +14,8 @@ poscar = Poscar.from_file(TEST_FILES_DIR / "POSCAR") ase = pytest.importorskip("ase") -from ase.constraints import FixAtoms # noqa: E402 -from ase.io import read # noqa: E402 +from ase.constraints import FixAtoms # noqa: E402 +from ase.io import read # noqa: E402 def test_get_atoms_from_structure(): From 32702675eb82097ec7fe1f7ccd88a282a64e616c Mon Sep 17 00:00:00 2001 From: Andrew Rosen Date: Wed, 27 Sep 2023 18:49:48 -1000 Subject: [PATCH 17/18] Clean up test suite more --- tests/io/test_ase.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index 4fddfa421e6..5529611de1c 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -248,6 +248,7 @@ def test_back_forth(structure_file): for k, v in atoms.todict().items(): assert str(atoms_back.todict()[k]) == str(v) +def test_back_forth_v2(): # Structure --> Atoms --> Structure --> Atoms structure = Structure.from_file(TEST_FILES_DIR / "POSCAR") structure.add_site_property("final_magmom", [1.0] * len(structure)) @@ -267,6 +268,7 @@ def test_back_forth(structure_file): d = jsanitize(structure, strict=True, enum_values=True) MontyDecoder().process_decoded(d) +def test_back_forth_v3(): # Atoms --> Molecule --> Atoms --> Molecule atoms = read(TEST_FILES_DIR / "acetylene.xyz") atoms.info = {"test": "hi"} @@ -282,6 +284,7 @@ def test_back_forth(structure_file): assert str(atoms_back.todict()[k]) == str(v) assert molecule_back == molecule +def test_back_forth_v4(): # Molecule --> Atoms --> Molecule --> Atoms molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz") molecule.set_charge_and_spin(-2, spin_multiplicity=3) From cc300c911edbe4bc6ca72a89857fc52640347e2d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 28 Sep 2023 04:51:19 +0000 Subject: [PATCH 18/18] pre-commit auto-fixes --- tests/io/test_ase.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/io/test_ase.py b/tests/io/test_ase.py index fd705df026a..bfcf36c5f67 100644 --- a/tests/io/test_ase.py +++ b/tests/io/test_ase.py @@ -248,6 +248,7 @@ def test_back_forth(structure_file): for k, v in atoms.todict().items(): assert str(atoms_back.todict()[k]) == str(v) + def test_back_forth_v2(): # Structure --> Atoms --> Structure --> Atoms structure = Structure.from_file(TEST_FILES_DIR / "POSCAR") @@ -268,6 +269,7 @@ def test_back_forth_v2(): d = jsanitize(structure, strict=True, enum_values=True) MontyDecoder().process_decoded(d) + def test_back_forth_v3(): # Atoms --> Molecule --> Atoms --> Molecule atoms = read(TEST_FILES_DIR / "acetylene.xyz") @@ -284,6 +286,7 @@ def test_back_forth_v3(): assert str(atoms_back.todict()[k]) == str(v) assert molecule_back == molecule + def test_back_forth_v4(): # Molecule --> Atoms --> Molecule --> Atoms molecule = Molecule.from_file(TEST_FILES_DIR / "acetylene.xyz")