From 1c24082f8aabd3213cffdf03c661b13ca985bf7e Mon Sep 17 00:00:00 2001 From: mikibonacci Date: Fri, 22 Nov 2024 11:39:01 +0000 Subject: [PATCH 1/9] Providing support for atomistic StructureData - orm.StructureData `to_atomistic` method and `has_atomistic` function - adapted `set_cell_from_structure` method for KpointsData - added tests for both structure and kpoints. --- src/aiida/orm/nodes/data/array/kpoints.py | 24 ++++--- src/aiida/orm/nodes/data/structure.py | 34 ++++++++++ tests/orm/nodes/data/test_kpoints.py | 77 +++++++++++++++++++++++ tests/test_dataclasses.py | 23 +++++++ 4 files changed, 150 insertions(+), 8 deletions(-) diff --git a/src/aiida/orm/nodes/data/array/kpoints.py b/src/aiida/orm/nodes/data/array/kpoints.py index ddf0cd6a96..8d43a39070 100644 --- a/src/aiida/orm/nodes/data/array/kpoints.py +++ b/src/aiida/orm/nodes/data/array/kpoints.py @@ -223,16 +223,24 @@ def set_cell_from_structure(self, structuredata): :param structuredata: an instance of StructureData """ - from aiida.orm import StructureData + from aiida.orm.nodes.data.structure import StructureData, has_atomistic if not isinstance(structuredata, StructureData): - raise ValueError( - 'An instance of StructureData should be passed to ' 'the KpointsData, found instead {}'.format( - structuredata.__class__ - ) - ) - cell = structuredata.cell - self.set_cell(cell, structuredata.pbc) + error_message = 'An instance of StructureData or aiida-atomistic StructureData should be passed to ' 'the KpointsData, found instead {}'.format( + structuredata.__class__ + ) + if has_atomistic: + from aiida_atomistic import StructureData as AtomisticStructureData + if not isinstance(structuredata, AtomisticStructureData): + raise ValueError(error_message) + else: + cell = structuredata.cell + self.set_cell(cell, structuredata.pbc) + else: + raise ValueError(error_message) + else: + cell = structuredata.cell + self.set_cell(cell, structuredata.pbc) def set_cell(self, cell, pbc=None): """Set a cell to be used for symmetry analysis. diff --git a/src/aiida/orm/nodes/data/structure.py b/src/aiida/orm/nodes/data/structure.py index f046898b52..6fcf479282 100644 --- a/src/aiida/orm/nodes/data/structure.py +++ b/src/aiida/orm/nodes/data/structure.py @@ -101,6 +101,14 @@ def has_pymatgen(): return False return True +def has_atomistic(): + """:return: True if the StructureData and StructureDataMutable from aiida-atomistic module can be imported, False otherwise.""" + try: + import aiida_atomistic # noqa: F401 + except ImportError: + return False + return True + def get_pymatgen_version(): """:return: string with pymatgen version, None if can not import.""" @@ -1876,6 +1884,32 @@ def _get_object_pymatgen_molecule(self, **kwargs): positions = [list(site.position) for site in self.sites] return Molecule(species, positions) + def to_atomistic(self): + """ + Returns the atomistic StructureData version of the orm.StructureData one. + """ + if not has_atomistic: + raise ImportError('aiida-atomistic plugin is not installed, \ + please install it to have full support for atomistic structures') + else: + from aiida_atomistic import StructureData as AtomisticStructureData + from aiida_atomistic import StructureDataMutable as AtomisticStructureDataMutable + + atomistic = AtomisticStructureDataMutable() + atomistic.set_pbc(self.pbc) + atomistic.set_cell(self.cell) + + for site in self.sites: + atomistic.add_atom( + **{ + 'symbols': self.get_kind(site.kind_name).symbol, + 'masses': self.get_kind(site.kind_name).mass, + 'positions': site.position, + 'kinds': site.kind_name, + } + ) + + return AtomisticStructureData.from_mutable(atomistic) class Kind: """This class contains the information about the species (kinds) of the system. diff --git a/tests/orm/nodes/data/test_kpoints.py b/tests/orm/nodes/data/test_kpoints.py index 112a46859e..67351bc1e2 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -11,7 +11,9 @@ import numpy as np import pytest from aiida.orm import KpointsData, StructureData, load_node +from aiida.orm.nodes.data.structure import has_atomistic +skip_atomistic = pytest.mark.skipif(not has_atomistic(), reason='Unable to import aiida-atomistic') class TestKpoints: """Test for the `Kpointsdata` class.""" @@ -81,3 +83,78 @@ def test_get_kpoints(self): kpt2 = load_node(kpt.pk) assert np.abs(kpt2.get_kpoints() - np.array(kpoints)).sum() == 0.0 assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 + +@skip_atomistic +class TestKpointsAtomisticStructureData: + """Test for the `Kpointsdata` class using the new atomistic StructureData.""" + + @pytest.fixture(autouse=True) + def init_profile(self): + """Initialize the profile.""" + + from aiida_atomistic import StructureDataMutable + from aiida_atomistic import StructureData as AtomisticStructureData + + alat = 5.430 # angstrom + cell = [ + [ + 0.5 * alat, + 0.5 * alat, + 0.0, + ], + [ + 0.0, + 0.5 * alat, + 0.5 * alat, + ], + [0.5 * alat, 0.0, 0.5 * alat], + ] + self.alat = alat + mutable = StructureDataMutable() + mutable.set_cell(cell) + mutable.add_atom(positions=(0.000 * alat, 0.000 * alat, 0.000 * alat), symbols='Si') + mutable.add_atom(positions=(0.250 * alat, 0.250 * alat, 0.250 * alat), symbols='Si') + self.structure = AtomisticStructureData.from_mutable(mutable) + # Define the expected reciprocal cell + val = 2.0 * np.pi / alat + self.expected_reciprocal_cell = np.array([[val, val, -val], [-val, val, val], [val, -val, val]]) + + def test_reciprocal_cell(self): + """Test the `reciprocal_cell` method. + + This is a regression test for #2749. + """ + kpt = KpointsData() + kpt.set_cell_from_structure(self.structure) + + assert np.abs(kpt.reciprocal_cell - self.expected_reciprocal_cell).sum() == 0.0 + + # Check also after storing + kpt.store() + kpt2 = load_node(kpt.pk) + assert np.abs(kpt2.reciprocal_cell - self.expected_reciprocal_cell).sum() == 0.0 + + def test_get_kpoints(self): + """Test the `get_kpoints` method.""" + kpt = KpointsData() + kpt.set_cell_from_structure(self.structure) + + kpoints = [ + [0.0, 0.0, 0.0], + [0.5, 0.5, 0.5], + ] + + cartesian_kpoints = [ + [0.0, 0.0, 0.0], + [np.pi / self.alat, np.pi / self.alat, np.pi / self.alat], + ] + + kpt.set_kpoints(kpoints) + assert np.abs(kpt.get_kpoints() - np.array(kpoints)).sum() == 0.0 + assert np.abs(kpt.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 + + # Check also after storing + kpt.store() + kpt2 = load_node(kpt.pk) + assert np.abs(kpt2.get_kpoints() - np.array(kpoints)).sum() == 0.0 + assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 \ No newline at end of file diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index 799b6454d7..da94111605 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -28,6 +28,7 @@ has_ase, has_pymatgen, has_spglib, + has_atomistic, ) @@ -67,6 +68,7 @@ def simplify(string): skip_spglib = pytest.mark.skipif(not has_spglib(), reason='Unable to import spglib') skip_pycifrw = pytest.mark.skipif(not has_pycifrw(), reason='Unable to import PyCifRW') skip_pymatgen = pytest.mark.skipif(not has_pymatgen(), reason='Unable to import pymatgen') +skip_atomistic = pytest.mark.skipif(not has_atomistic(), reason='Unable to import aiida-atomistic') @skip_pymatgen @@ -1850,6 +1852,27 @@ def test_clone(self): for i in range(3): assert round(abs(c.sites[1].position[i] - 1.0), 7) == 0 +@skip_atomistic +class TestAtomisticStructureData: + """Tests the creation of StructureData objects (cell and pbc), and its conversion to the new atomistic StructureData.""" + + def test_to_atomistic(self): + """Test the conversion to the atomistic structure.""" + + # Create a structure with a single atom + from aiida_atomistic import StructureData as AtomisticStructureData + + legacy = StructureData(cell=((1.0, 0.0, 0.0), (0.0, 2.0, 0.0), (0.0, 0.0, 3.0))) + legacy.append_atom(position=(0.0, 0.0, 0.0), symbols=['Ba'], name='Ba1') + + # Convert to atomistic structure + structure = legacy.to_atomistic() + + # Check that the structure is as expected + assert isinstance(structure, AtomisticStructureData) + assert structure.properties.sites[0].kinds == legacy.sites[0].kind_name + assert structure.properties.sites[0].positions == list(legacy.sites[0].position) + assert structure.properties.cell == legacy.cell class TestStructureDataFromAse: """Tests the creation of Sites from/to a ASE object.""" From 21897d99565492271b7494248b4f13c8fafba5ad Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 22 Nov 2024 11:41:32 +0000 Subject: [PATCH 2/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/aiida/orm/nodes/data/array/kpoints.py | 10 ++++++---- src/aiida/orm/nodes/data/structure.py | 16 ++++++++++------ tests/orm/nodes/data/test_kpoints.py | 10 ++++++---- tests/test_dataclasses.py | 4 +++- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/aiida/orm/nodes/data/array/kpoints.py b/src/aiida/orm/nodes/data/array/kpoints.py index 8d43a39070..86525fde4b 100644 --- a/src/aiida/orm/nodes/data/array/kpoints.py +++ b/src/aiida/orm/nodes/data/array/kpoints.py @@ -226,11 +226,13 @@ def set_cell_from_structure(self, structuredata): from aiida.orm.nodes.data.structure import StructureData, has_atomistic if not isinstance(structuredata, StructureData): - error_message = 'An instance of StructureData or aiida-atomistic StructureData should be passed to ' 'the KpointsData, found instead {}'.format( - structuredata.__class__ - ) + error_message = ( + 'An instance of StructureData or aiida-atomistic StructureData should be passed to ' + 'the KpointsData, found instead {}'.format(structuredata.__class__) + ) if has_atomistic: from aiida_atomistic import StructureData as AtomisticStructureData + if not isinstance(structuredata, AtomisticStructureData): raise ValueError(error_message) else: @@ -238,7 +240,7 @@ def set_cell_from_structure(self, structuredata): self.set_cell(cell, structuredata.pbc) else: raise ValueError(error_message) - else: + else: cell = structuredata.cell self.set_cell(cell, structuredata.pbc) diff --git a/src/aiida/orm/nodes/data/structure.py b/src/aiida/orm/nodes/data/structure.py index 6fcf479282..f06d0c22dc 100644 --- a/src/aiida/orm/nodes/data/structure.py +++ b/src/aiida/orm/nodes/data/structure.py @@ -101,6 +101,7 @@ def has_pymatgen(): return False return True + def has_atomistic(): """:return: True if the StructureData and StructureDataMutable from aiida-atomistic module can be imported, False otherwise.""" try: @@ -1889,16 +1890,18 @@ def to_atomistic(self): Returns the atomistic StructureData version of the orm.StructureData one. """ if not has_atomistic: - raise ImportError('aiida-atomistic plugin is not installed, \ - please install it to have full support for atomistic structures') + raise ImportError( + 'aiida-atomistic plugin is not installed, \ + please install it to have full support for atomistic structures' + ) else: from aiida_atomistic import StructureData as AtomisticStructureData from aiida_atomistic import StructureDataMutable as AtomisticStructureDataMutable - + atomistic = AtomisticStructureDataMutable() atomistic.set_pbc(self.pbc) atomistic.set_cell(self.cell) - + for site in self.sites: atomistic.add_atom( **{ @@ -1906,11 +1909,12 @@ def to_atomistic(self): 'masses': self.get_kind(site.kind_name).mass, 'positions': site.position, 'kinds': site.kind_name, - } + } ) - + return AtomisticStructureData.from_mutable(atomistic) + class Kind: """This class contains the information about the species (kinds) of the system. diff --git a/tests/orm/nodes/data/test_kpoints.py b/tests/orm/nodes/data/test_kpoints.py index 67351bc1e2..b902e50b48 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -15,6 +15,7 @@ skip_atomistic = pytest.mark.skipif(not has_atomistic(), reason='Unable to import aiida-atomistic') + class TestKpoints: """Test for the `Kpointsdata` class.""" @@ -84,6 +85,7 @@ def test_get_kpoints(self): assert np.abs(kpt2.get_kpoints() - np.array(kpoints)).sum() == 0.0 assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 + @skip_atomistic class TestKpointsAtomisticStructureData: """Test for the `Kpointsdata` class using the new atomistic StructureData.""" @@ -91,10 +93,10 @@ class TestKpointsAtomisticStructureData: @pytest.fixture(autouse=True) def init_profile(self): """Initialize the profile.""" - - from aiida_atomistic import StructureDataMutable + from aiida_atomistic import StructureData as AtomisticStructureData - + from aiida_atomistic import StructureDataMutable + alat = 5.430 # angstrom cell = [ [ @@ -157,4 +159,4 @@ def test_get_kpoints(self): kpt.store() kpt2 = load_node(kpt.pk) assert np.abs(kpt2.get_kpoints() - np.array(kpoints)).sum() == 0.0 - assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 \ No newline at end of file + assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index da94111605..a0621e431f 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -26,9 +26,9 @@ get_formula, get_pymatgen_version, has_ase, + has_atomistic, has_pymatgen, has_spglib, - has_atomistic, ) @@ -1852,6 +1852,7 @@ def test_clone(self): for i in range(3): assert round(abs(c.sites[1].position[i] - 1.0), 7) == 0 + @skip_atomistic class TestAtomisticStructureData: """Tests the creation of StructureData objects (cell and pbc), and its conversion to the new atomistic StructureData.""" @@ -1874,6 +1875,7 @@ def test_to_atomistic(self): assert structure.properties.sites[0].positions == list(legacy.sites[0].position) assert structure.properties.cell == legacy.cell + class TestStructureDataFromAse: """Tests the creation of Sites from/to a ASE object.""" From b9d739ca6152324ffc00d41418ac8d96aa6f08bd Mon Sep 17 00:00:00 2001 From: Miki Bonacci Date: Fri, 22 Nov 2024 20:52:00 +0100 Subject: [PATCH 3/9] Bugfix `has_atomistic`. --- src/aiida/orm/nodes/data/array/kpoints.py | 2 +- src/aiida/orm/nodes/data/structure.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/aiida/orm/nodes/data/array/kpoints.py b/src/aiida/orm/nodes/data/array/kpoints.py index 86525fde4b..b4dadf2a85 100644 --- a/src/aiida/orm/nodes/data/array/kpoints.py +++ b/src/aiida/orm/nodes/data/array/kpoints.py @@ -230,7 +230,7 @@ def set_cell_from_structure(self, structuredata): 'An instance of StructureData or aiida-atomistic StructureData should be passed to ' 'the KpointsData, found instead {}'.format(structuredata.__class__) ) - if has_atomistic: + if has_atomistic(): from aiida_atomistic import StructureData as AtomisticStructureData if not isinstance(structuredata, AtomisticStructureData): diff --git a/src/aiida/orm/nodes/data/structure.py b/src/aiida/orm/nodes/data/structure.py index f06d0c22dc..875736f4c3 100644 --- a/src/aiida/orm/nodes/data/structure.py +++ b/src/aiida/orm/nodes/data/structure.py @@ -1889,7 +1889,7 @@ def to_atomistic(self): """ Returns the atomistic StructureData version of the orm.StructureData one. """ - if not has_atomistic: + if not has_atomistic(): raise ImportError( 'aiida-atomistic plugin is not installed, \ please install it to have full support for atomistic structures' From 87032b4775711a46e18d690ebd6332e2000d5236 Mon Sep 17 00:00:00 2001 From: Miki Bonacci Date: Mon, 25 Nov 2024 21:51:58 +0100 Subject: [PATCH 4/9] Improving of the implementation --- src/aiida/orm/nodes/data/structure.py | 19 ++++++++---------- tests/orm/nodes/data/test_kpoints.py | 5 ++--- tests/test_dataclasses.py | 29 ++++++++++++--------------- 3 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/aiida/orm/nodes/data/structure.py b/src/aiida/orm/nodes/data/structure.py index 875736f4c3..b66868a106 100644 --- a/src/aiida/orm/nodes/data/structure.py +++ b/src/aiida/orm/nodes/data/structure.py @@ -102,7 +102,7 @@ def has_pymatgen(): return True -def has_atomistic(): +def has_atomistic() -> bool: """:return: True if the StructureData and StructureDataMutable from aiida-atomistic module can be imported, False otherwise.""" try: import aiida_atomistic # noqa: F401 @@ -1895,24 +1895,21 @@ def to_atomistic(self): please install it to have full support for atomistic structures' ) else: - from aiida_atomistic import StructureData as AtomisticStructureData - from aiida_atomistic import StructureDataMutable as AtomisticStructureDataMutable + from aiida_atomistic import StructureData, StructureDataMutable - atomistic = AtomisticStructureDataMutable() + atomistic = StructureDataMutable() atomistic.set_pbc(self.pbc) atomistic.set_cell(self.cell) for site in self.sites: atomistic.add_atom( - **{ - 'symbols': self.get_kind(site.kind_name).symbol, - 'masses': self.get_kind(site.kind_name).mass, - 'positions': site.position, - 'kinds': site.kind_name, - } + symbols= self.get_kind(site.kind_name).symbol, + masses= self.get_kind(site.kind_name).mass, + positions= site.position, + kinds= site.kind_name, ) - return AtomisticStructureData.from_mutable(atomistic) + return StructureData.from_mutable(atomistic) class Kind: diff --git a/tests/orm/nodes/data/test_kpoints.py b/tests/orm/nodes/data/test_kpoints.py index b902e50b48..16019c9eb7 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -94,8 +94,7 @@ class TestKpointsAtomisticStructureData: def init_profile(self): """Initialize the profile.""" - from aiida_atomistic import StructureData as AtomisticStructureData - from aiida_atomistic import StructureDataMutable + from aiida_atomistic import StructureData, StructureDataMutable alat = 5.430 # angstrom cell = [ @@ -116,7 +115,7 @@ def init_profile(self): mutable.set_cell(cell) mutable.add_atom(positions=(0.000 * alat, 0.000 * alat, 0.000 * alat), symbols='Si') mutable.add_atom(positions=(0.250 * alat, 0.250 * alat, 0.250 * alat), symbols='Si') - self.structure = AtomisticStructureData.from_mutable(mutable) + self.structure = StructureData.from_mutable(mutable) # Define the expected reciprocal cell val = 2.0 * np.pi / alat self.expected_reciprocal_cell = np.array([[val, val, -val], [-val, val, val], [val, -val, val]]) diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index a0621e431f..1ea06f0569 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -1854,26 +1854,23 @@ def test_clone(self): @skip_atomistic -class TestAtomisticStructureData: - """Tests the creation of StructureData objects (cell and pbc), and its conversion to the new atomistic StructureData.""" +def test_to_atomistic(self): + """Test the conversion from orm.StructureData to the atomistic structure.""" - def test_to_atomistic(self): - """Test the conversion to the atomistic structure.""" + # Create a structure with a single atom + from aiida_atomistic import StructureData as AtomisticStructureData - # Create a structure with a single atom - from aiida_atomistic import StructureData as AtomisticStructureData + legacy = StructureData(cell=((1.0, 0.0, 0.0), (0.0, 2.0, 0.0), (0.0, 0.0, 3.0))) + legacy.append_atom(position=(0.0, 0.0, 0.0), symbols=['Ba'], name='Ba1') - legacy = StructureData(cell=((1.0, 0.0, 0.0), (0.0, 2.0, 0.0), (0.0, 0.0, 3.0))) - legacy.append_atom(position=(0.0, 0.0, 0.0), symbols=['Ba'], name='Ba1') + # Convert to atomistic structure + structure = legacy.to_atomistic() - # Convert to atomistic structure - structure = legacy.to_atomistic() - - # Check that the structure is as expected - assert isinstance(structure, AtomisticStructureData) - assert structure.properties.sites[0].kinds == legacy.sites[0].kind_name - assert structure.properties.sites[0].positions == list(legacy.sites[0].position) - assert structure.properties.cell == legacy.cell + # Check that the structure is as expected + assert isinstance(structure, AtomisticStructureData) + assert structure.properties.sites[0].kinds == legacy.sites[0].kind_name + assert structure.properties.sites[0].positions == list(legacy.sites[0].position) + assert structure.properties.cell == legacy.cell class TestStructureDataFromAse: From f45ee3052863abc0c8d51f32561c0c44f3563512 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 20:53:11 +0000 Subject: [PATCH 5/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/aiida/orm/nodes/data/structure.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/aiida/orm/nodes/data/structure.py b/src/aiida/orm/nodes/data/structure.py index b66868a106..a57abfc078 100644 --- a/src/aiida/orm/nodes/data/structure.py +++ b/src/aiida/orm/nodes/data/structure.py @@ -1903,10 +1903,10 @@ def to_atomistic(self): for site in self.sites: atomistic.add_atom( - symbols= self.get_kind(site.kind_name).symbol, - masses= self.get_kind(site.kind_name).mass, - positions= site.position, - kinds= site.kind_name, + symbols=self.get_kind(site.kind_name).symbol, + masses=self.get_kind(site.kind_name).mass, + positions=site.position, + kinds=site.kind_name, ) return StructureData.from_mutable(atomistic) From f44eddf4c8d7d9f645ddbbff8e7ff58798ccbaa2 Mon Sep 17 00:00:00 2001 From: Miki Bonacci Date: Fri, 29 Nov 2024 11:29:18 +0100 Subject: [PATCH 6/9] Simplification of the logic for Legacy or atomistic Structure. both in kpoints.py and test_kpoints.py --- src/aiida/orm/nodes/data/array/kpoints.py | 25 +++--- tests/orm/nodes/data/test_kpoints.py | 100 ++++------------------ 2 files changed, 26 insertions(+), 99 deletions(-) diff --git a/src/aiida/orm/nodes/data/array/kpoints.py b/src/aiida/orm/nodes/data/array/kpoints.py index b4dadf2a85..3cd9757c30 100644 --- a/src/aiida/orm/nodes/data/array/kpoints.py +++ b/src/aiida/orm/nodes/data/array/kpoints.py @@ -223,23 +223,18 @@ def set_cell_from_structure(self, structuredata): :param structuredata: an instance of StructureData """ - from aiida.orm.nodes.data.structure import StructureData, has_atomistic + from aiida.orm.nodes.data.structure import has_atomistic, StructureData as LegacyStructureData + + if has_atomistic(): + structures_classes = (StructureData, LegacyStructureData) + else: + structures_classes = (LegacyStructureData,) - if not isinstance(structuredata, StructureData): - error_message = ( - 'An instance of StructureData or aiida-atomistic StructureData should be passed to ' - 'the KpointsData, found instead {}'.format(structuredata.__class__) + if not isinstance(structuredata, structures_classes): + raise TypeError( + f'An instance of {structures_classes} should be passed to ' + f'the KpointsData, found instead {type(structuredata)}' ) - if has_atomistic(): - from aiida_atomistic import StructureData as AtomisticStructureData - - if not isinstance(structuredata, AtomisticStructureData): - raise ValueError(error_message) - else: - cell = structuredata.cell - self.set_cell(cell, structuredata.pbc) - else: - raise ValueError(error_message) else: cell = structuredata.cell self.set_cell(cell, structuredata.pbc) diff --git a/tests/orm/nodes/data/test_kpoints.py b/tests/orm/nodes/data/test_kpoints.py index 16019c9eb7..c80ef80c97 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -10,18 +10,22 @@ import numpy as np import pytest -from aiida.orm import KpointsData, StructureData, load_node +from aiida.orm import KpointsData, load_node, StructureData as LegacyStructureData from aiida.orm.nodes.data.structure import has_atomistic -skip_atomistic = pytest.mark.skipif(not has_atomistic(), reason='Unable to import aiida-atomistic') - - +if not has_atomistic(): + structures_classes = [LegacyStructureData,] +else: + from aiida_atomistic import StructureData + structures_classes = [LegacyStructureData, StructureData] + +@pytest.mark.parametrize('structure_class', structures_classes) class TestKpoints: """Test for the `Kpointsdata` class.""" @pytest.fixture(autouse=True) - def init_profile(self): - """Initialize the profile.""" + def generate_structure(self, structure_class): + """Generate the StructureData.""" alat = 5.430 # angstrom cell = [ [ @@ -37,85 +41,13 @@ def init_profile(self): [0.5 * alat, 0.0, 0.5 * alat], ] self.alat = alat - structure = StructureData(cell=cell) + structure = LegacyStructureData(cell=cell) structure.append_atom(position=(0.000 * alat, 0.000 * alat, 0.000 * alat), symbols=['Si']) structure.append_atom(position=(0.250 * alat, 0.250 * alat, 0.250 * alat), symbols=['Si']) - self.structure = structure - # Define the expected reciprocal cell - val = 2.0 * np.pi / alat - self.expected_reciprocal_cell = np.array([[val, val, -val], [-val, val, val], [val, -val, val]]) - - def test_reciprocal_cell(self): - """Test the `reciprocal_cell` method. - - This is a regression test for #2749. - """ - kpt = KpointsData() - kpt.set_cell_from_structure(self.structure) - - assert np.abs(kpt.reciprocal_cell - self.expected_reciprocal_cell).sum() == 0.0 - - # Check also after storing - kpt.store() - kpt2 = load_node(kpt.pk) - assert np.abs(kpt2.reciprocal_cell - self.expected_reciprocal_cell).sum() == 0.0 - - def test_get_kpoints(self): - """Test the `get_kpoints` method.""" - kpt = KpointsData() - kpt.set_cell_from_structure(self.structure) - - kpoints = [ - [0.0, 0.0, 0.0], - [0.5, 0.5, 0.5], - ] - - cartesian_kpoints = [ - [0.0, 0.0, 0.0], - [np.pi / self.alat, np.pi / self.alat, np.pi / self.alat], - ] - - kpt.set_kpoints(kpoints) - assert np.abs(kpt.get_kpoints() - np.array(kpoints)).sum() == 0.0 - assert np.abs(kpt.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 - - # Check also after storing - kpt.store() - kpt2 = load_node(kpt.pk) - assert np.abs(kpt2.get_kpoints() - np.array(kpoints)).sum() == 0.0 - assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 - - -@skip_atomistic -class TestKpointsAtomisticStructureData: - """Test for the `Kpointsdata` class using the new atomistic StructureData.""" - - @pytest.fixture(autouse=True) - def init_profile(self): - """Initialize the profile.""" - - from aiida_atomistic import StructureData, StructureDataMutable - - alat = 5.430 # angstrom - cell = [ - [ - 0.5 * alat, - 0.5 * alat, - 0.0, - ], - [ - 0.0, - 0.5 * alat, - 0.5 * alat, - ], - [0.5 * alat, 0.0, 0.5 * alat], - ] - self.alat = alat - mutable = StructureDataMutable() - mutable.set_cell(cell) - mutable.add_atom(positions=(0.000 * alat, 0.000 * alat, 0.000 * alat), symbols='Si') - mutable.add_atom(positions=(0.250 * alat, 0.250 * alat, 0.250 * alat), symbols='Si') - self.structure = StructureData.from_mutable(mutable) + if structure_class == LegacyStructureData: + self.structure = structure + else: + self.structure = LegacyStructureData.to_atomistic(structure) # Define the expected reciprocal cell val = 2.0 * np.pi / alat self.expected_reciprocal_cell = np.array([[val, val, -val], [-val, val, val], [val, -val, val]]) @@ -158,4 +90,4 @@ def test_get_kpoints(self): kpt.store() kpt2 = load_node(kpt.pk) assert np.abs(kpt2.get_kpoints() - np.array(kpoints)).sum() == 0.0 - assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 + assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 \ No newline at end of file From 81eeb08b776b3f3d132794a3f5f07e008ff878b7 Mon Sep 17 00:00:00 2001 From: Miki Bonacci Date: Fri, 29 Nov 2024 11:45:21 +0100 Subject: [PATCH 7/9] Small bugfix and changing order if_else in set_cell_from_structure --- src/aiida/orm/nodes/data/array/kpoints.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/aiida/orm/nodes/data/array/kpoints.py b/src/aiida/orm/nodes/data/array/kpoints.py index 3cd9757c30..c048d56eb4 100644 --- a/src/aiida/orm/nodes/data/array/kpoints.py +++ b/src/aiida/orm/nodes/data/array/kpoints.py @@ -223,12 +223,15 @@ def set_cell_from_structure(self, structuredata): :param structuredata: an instance of StructureData """ - from aiida.orm.nodes.data.structure import has_atomistic, StructureData as LegacyStructureData - - if has_atomistic(): - structures_classes = (StructureData, LegacyStructureData) + from aiida.orm.nodes.data.structure import StructureData as LegacyStructureData + from aiida.orm.nodes.data.structure import has_atomistic + + if not has_atomistic(): + structures_classes = (LegacyStructureData,) # type: tuple else: - structures_classes = (LegacyStructureData,) + from aiida_atomistic import StructureData # type: ignore[import-untyped] + + structures_classes = (LegacyStructureData, StructureData) if not isinstance(structuredata, structures_classes): raise TypeError( From 2b05eed301da940dfc7b8693eda0de9b8e2e3e91 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 29 Nov 2024 10:46:07 +0000 Subject: [PATCH 8/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/orm/nodes/data/test_kpoints.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/orm/nodes/data/test_kpoints.py b/tests/orm/nodes/data/test_kpoints.py index c80ef80c97..85c5eac98f 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -10,15 +10,20 @@ import numpy as np import pytest -from aiida.orm import KpointsData, load_node, StructureData as LegacyStructureData +from aiida.orm import KpointsData, load_node +from aiida.orm import StructureData as LegacyStructureData from aiida.orm.nodes.data.structure import has_atomistic if not has_atomistic(): - structures_classes = [LegacyStructureData,] + structures_classes = [ + LegacyStructureData, + ] else: from aiida_atomistic import StructureData + structures_classes = [LegacyStructureData, StructureData] - + + @pytest.mark.parametrize('structure_class', structures_classes) class TestKpoints: """Test for the `Kpointsdata` class.""" @@ -90,4 +95,4 @@ def test_get_kpoints(self): kpt.store() kpt2 = load_node(kpt.pk) assert np.abs(kpt2.get_kpoints() - np.array(kpoints)).sum() == 0.0 - assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 \ No newline at end of file + assert np.abs(kpt2.get_kpoints(cartesian=True) - np.array(cartesian_kpoints)).sum() == 0.0 From fe6cbcc34d453535ee771311281ba8a19a43efbe Mon Sep 17 00:00:00 2001 From: Miki Bonacci Date: Fri, 29 Nov 2024 12:04:26 +0100 Subject: [PATCH 9/9] Full parametrization of test_kpoints.py for atomistic case. --- tests/orm/nodes/data/test_kpoints.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/orm/nodes/data/test_kpoints.py b/tests/orm/nodes/data/test_kpoints.py index 85c5eac98f..a8461e9a69 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -14,12 +14,12 @@ from aiida.orm import StructureData as LegacyStructureData from aiida.orm.nodes.data.structure import has_atomistic +skip_atomistic = pytest.mark.skipif(not has_atomistic(), reason='aiida-atomistic not installed') + if not has_atomistic(): - structures_classes = [ - LegacyStructureData, - ] + structures_classes = [LegacyStructureData, pytest.param('StructureData', marks=skip_atomistic)] else: - from aiida_atomistic import StructureData + from aiida_atomistic import StructureData # type: ignore[import-untyped] structures_classes = [LegacyStructureData, StructureData]