From 5351dbc96eb755a43b48f50f708e1038cb4116b0 Mon Sep 17 00:00:00 2001 From: mikibonacci Date: Fri, 22 Nov 2024 11:39:01 +0000 Subject: [PATCH 1/6] 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 e7958970a4..01e8098e2c 100644 --- a/src/aiida/orm/nodes/data/array/kpoints.py +++ b/src/aiida/orm/nodes/data/array/kpoints.py @@ -190,16 +190,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 0e3e09ef5b..e431328893 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 has_spglib(): """:return: True if the spglib module can be imported, False otherwise.""" @@ -1865,6 +1873,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 73d25f82ea..6d92d5cf46 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -12,7 +12,9 @@ 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.""" @@ -82,3 +84,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 df0a43c195..406242d3e6 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') class TestCifData: @@ -1847,6 +1849,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 03fc8e8b81e30fca9efc6ab1eee85da5daee5293 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/6] [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 01e8098e2c..f2eb0414eb 100644 --- a/src/aiida/orm/nodes/data/array/kpoints.py +++ b/src/aiida/orm/nodes/data/array/kpoints.py @@ -193,11 +193,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: @@ -205,7 +207,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 e431328893..a1211ee2e3 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: @@ -1878,16 +1879,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( **{ @@ -1895,11 +1898,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 6d92d5cf46..c8f59ccdb6 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -16,6 +16,7 @@ skip_atomistic = pytest.mark.skipif(not has_atomistic(), reason='Unable to import aiida-atomistic') + class TestKpoints: """Test for the `Kpointsdata` class.""" @@ -85,6 +86,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.""" @@ -92,10 +94,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 = [ [ @@ -158,4 +160,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 406242d3e6..50713bced4 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -26,9 +26,9 @@ ase_refine_cell, get_formula, has_ase, + has_atomistic, has_pymatgen, has_spglib, - has_atomistic, ) @@ -1849,6 +1849,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.""" @@ -1871,6 +1872,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 57658f39f6ecb720ada7dc92368eba136267f0c1 Mon Sep 17 00:00:00 2001 From: Miki Bonacci Date: Fri, 22 Nov 2024 20:52:00 +0100 Subject: [PATCH 3/6] 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 f2eb0414eb..c5ad5eea20 100644 --- a/src/aiida/orm/nodes/data/array/kpoints.py +++ b/src/aiida/orm/nodes/data/array/kpoints.py @@ -197,7 +197,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 a1211ee2e3..0ccfe21cec 100644 --- a/src/aiida/orm/nodes/data/structure.py +++ b/src/aiida/orm/nodes/data/structure.py @@ -1878,7 +1878,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 6359e4e0604c9fcb96019c5735e38a15ba8616d9 Mon Sep 17 00:00:00 2001 From: Miki Bonacci Date: Mon, 25 Nov 2024 21:51:58 +0100 Subject: [PATCH 4/6] 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 0ccfe21cec..8506947da9 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 @@ -1884,24 +1884,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 c8f59ccdb6..35ece0856f 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -95,8 +95,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 = [ @@ -117,7 +116,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 50713bced4..8a8cfb6ee7 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -1851,26 +1851,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 e380f9a0b092ecd160373768364fdf7693da913b 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/6] [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 8506947da9..6b439330b4 100644 --- a/src/aiida/orm/nodes/data/structure.py +++ b/src/aiida/orm/nodes/data/structure.py @@ -1892,10 +1892,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 0f6b51b9fcf6c756d38949668f162c2d34ed73f1 Mon Sep 17 00:00:00 2001 From: Miki Bonacci Date: Fri, 29 Nov 2024 11:29:18 +0100 Subject: [PATCH 6/6] Simplification of the logic for Legacy or atomistic Structure. both in kpoints.py and test_kpoints.py Small bugfix and changing order if_else in set_cell_from_structure [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Full parametrization of test_kpoints.py for atomistic case. Adding NotImplementedError in seekpath_structure_analysis. To be implemented soon, to make bands.py work. [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Fixing pre-commit problem Fixing docstring for aiida-atomistic Support CifData conversion CLI importer support for atomistic modifying the to_atomistic method to work with the new atomistic API --- .../commands/cmd_data/cmd_structure.py | 24 ++++- src/aiida/orm/nodes/data/array/kpoints.py | 28 +++--- src/aiida/orm/nodes/data/structure.py | 22 ++--- src/aiida/tools/data/structure.py | 19 +++- tests/orm/nodes/data/test_kpoints.py | 99 ++++--------------- 5 files changed, 80 insertions(+), 112 deletions(-) diff --git a/src/aiida/cmdline/commands/cmd_data/cmd_structure.py b/src/aiida/cmdline/commands/cmd_data/cmd_structure.py index d42b8c426d..10853ec70a 100644 --- a/src/aiida/cmdline/commands/cmd_data/cmd_structure.py +++ b/src/aiida/cmdline/commands/cmd_data/cmd_structure.py @@ -185,10 +185,17 @@ def structure_import(): help='Set periodic boundary conditions for each lattice direction, where 0 means periodic and 1 means periodic.', ) @click.option('--label', type=click.STRING, show_default=False, help='Set the structure node label (empty by default)') +@click.option( + '--to_atomistic', + type=click.BOOL, + default=False, + show_default=True, + help='Set the structure node as atomistic StructureData (default is False)', +) @options.GROUP() @options.DRY_RUN() @decorators.with_dbenv() -def import_aiida_xyz(filename, vacuum_factor, vacuum_addition, pbc, label, group, dry_run): +def import_aiida_xyz(filename, vacuum_factor, vacuum_addition, pbc, label, to_atomistic, group, dry_run): """Import structure in XYZ format using AiiDA's internal importer""" from aiida.orm import StructureData @@ -215,6 +222,9 @@ def import_aiida_xyz(filename, vacuum_factor, vacuum_addition, pbc, label, group if label: new_structure.label = label + if to_atomistic: + new_structure = new_structure.to_atomistic() + _store_structure(new_structure, dry_run) if group: @@ -224,10 +234,17 @@ def import_aiida_xyz(filename, vacuum_factor, vacuum_addition, pbc, label, group @structure_import.command('ase') @click.argument('filename', type=click.Path(exists=True, dir_okay=False, resolve_path=True)) @click.option('--label', type=click.STRING, show_default=False, help='Set the structure node label (empty by default)') +@click.option( + '--to_atomistic', + type=click.BOOL, + default=False, + show_default=True, + help='Set the structure node as atomistic StructureData (default is False)', +) @options.GROUP() @options.DRY_RUN() @decorators.with_dbenv() -def import_ase(filename, label, group, dry_run): +def import_ase(filename, label, to_atomistic, group, dry_run): """Import structure with the ase library that supports a number of different formats""" from aiida.orm import StructureData @@ -245,6 +262,9 @@ def import_ase(filename, label, group, dry_run): if label: new_structure.label = label + if to_atomistic: + new_structure = new_structure.to_atomistic() + _store_structure(new_structure, dry_run) if group: diff --git a/src/aiida/orm/nodes/data/array/kpoints.py b/src/aiida/orm/nodes/data/array/kpoints.py index c5ad5eea20..3bc5995d39 100644 --- a/src/aiida/orm/nodes/data/array/kpoints.py +++ b/src/aiida/orm/nodes/data/array/kpoints.py @@ -190,23 +190,21 @@ 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 StructureData as LegacyStructureData + from aiida.orm.nodes.data.structure import 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__) + if not has_atomistic(): + structures_classes = (LegacyStructureData,) # type: tuple + else: + from aiida_atomistic import StructureData # type: ignore[import-untyped] + + structures_classes = (LegacyStructureData, StructureData) + + 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/src/aiida/orm/nodes/data/structure.py b/src/aiida/orm/nodes/data/structure.py index 6b439330b4..7342caff38 100644 --- a/src/aiida/orm/nodes/data/structure.py +++ b/src/aiida/orm/nodes/data/structure.py @@ -103,7 +103,7 @@ def has_pymatgen(): def has_atomistic() -> bool: - """:return: True if the StructureData and StructureDataMutable from aiida-atomistic module can be imported, False otherwise.""" + """:return: True if theaiida-atomistic module can be imported, False otherwise.""" try: import aiida_atomistic # noqa: F401 except ImportError: @@ -1884,21 +1884,21 @@ def to_atomistic(self): please install it to have full support for atomistic structures' ) else: - from aiida_atomistic import StructureData, StructureDataMutable + from aiida_atomistic import StructureData, StructureBuilder - atomistic = StructureDataMutable() + atomistic = StructureBuilder() 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 StructureData.from_mutable(atomistic) + atomistic.append_atom({ + 'symbol': self.get_kind(site.kind_name).symbol, + 'mass': self.get_kind(site.kind_name).mass, + 'position': site.position, + 'kind_name': site.kind_name, + }) + + return StructureData.from_builder(atomistic) class Kind: diff --git a/src/aiida/tools/data/structure.py b/src/aiida/tools/data/structure.py index 92d02e504c..0bed8b09cf 100644 --- a/src/aiida/tools/data/structure.py +++ b/src/aiida/tools/data/structure.py @@ -17,9 +17,21 @@ import numpy as np +from aiida.common import exceptions from aiida.common.constants import elements from aiida.engine import calcfunction -from aiida.orm.nodes.data.structure import Kind, Site, StructureData +from aiida.orm.nodes.data.structure import Kind, Site +from aiida.orm.nodes.data.structure import StructureData as LegacyStructureData +from aiida.plugins import DataFactory + +try: + StructureData = DataFactory('atomistic.structure') + HAS_ATOMISTIC = True +except exceptions.MissingEntryPointError: + structures_classes = (LegacyStructureData,) + HAS_ATOMISTIC = False +else: + structures_classes = (LegacyStructureData, StructureData) # type: ignore[assignment] __all__ = ('spglib_tuple_to_structure', 'structure_to_spglib_tuple') @@ -35,7 +47,8 @@ def _get_cif_ase_inline(struct, parameters): kwargs = {} if parameters is not None: kwargs = parameters.get_dict() - cif = CifData(ase=struct.get_ase(**kwargs)) + ase_structure = struct.get_ase(**kwargs) if isinstance(struct, LegacyStructureData) else struct.to_ase(**kwargs) + cif = CifData(ase=ase_structure) formula = struct.get_formula(mode='hill', separator=' ') for i in cif.values.keys(): cif.values[i]['_symmetry_space_group_name_H-M'] = 'P 1' @@ -152,7 +165,7 @@ def spglib_tuple_to_structure(structure_tuple, kind_info=None, kinds=None): except KeyError as exc: raise ValueError(f'Unable to find kind in kind_info for number {exc.args[0]}') - structure = StructureData(cell=cell) + structure = LegacyStructureData(cell=cell) for k in _kinds: structure.append_kind(k) abs_pos = np.dot(rel_pos, cell) diff --git a/tests/orm/nodes/data/test_kpoints.py b/tests/orm/nodes/data/test_kpoints.py index 35ece0856f..98f674bc79 100644 --- a/tests/orm/nodes/data/test_kpoints.py +++ b/tests/orm/nodes/data/test_kpoints.py @@ -11,18 +11,27 @@ import numpy as np import pytest -from aiida.orm import KpointsData, StructureData, load_node +from aiida.orm import KpointsData, load_node +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='Unable to import aiida-atomistic') +skip_atomistic = pytest.mark.skipif(not has_atomistic(), reason='aiida-atomistic not installed') +if not has_atomistic(): + structures_classes = [LegacyStructureData, pytest.param('StructureData', marks=skip_atomistic)] +else: + from aiida_atomistic import StructureData # type: ignore[import-untyped] + 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 = [ [ @@ -38,85 +47,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]])