Merge branch 'main' into fix/xps/molecule

.github/workflows/update_changelog.py

@@ -30,6 +30,9 @@
 ### ⬆️ Update dependencies
 
 
+### 🧪 Tests
+
+
 ### ♻️ Refactor
 
 """
@@ -73,7 +76,7 @@ def update_changelog():
         # Add the commit hash (short) to link to the changelog
         commit = commit.strip("'")
         hash_short, hash_long, message = commit.split('|', maxsplit=2)
-        message += f'[[{hash_short}](https://github.com/aiidateam/aiida-quantumespresso/commit/{hash_long})]'
+        message += f' [[{hash_short}](https://github.com/aiidateam/aiida-quantumespresso/commit/{hash_long})]'
         changelog_message += f'\n* {message}'
 
     with Path('CHANGELOG.md').open('w', encoding='utf8') as handle:

CHANGELOG.md

@@ -1,34 +1,127 @@
+## v4.5.1
+
+This patch release fixes some issues with the changes introduced in [b9c7517](https://github.com/aiidateam/aiida-quantumespresso/commit/b9c7517744e645a93d4afc9b1999881fc39a0e46) and released in v4.5.0.
+The new approach for setting the q-points was unfortunately broken, which is now fixed in [c353cc2](https://github.com/aiidateam/aiida-quantumespresso/commit/c353cc2e4104352ef9b5490adb53a60da47f293d).
+Moreover, the validation that was added to the top-level inputs of the `PhBaseWorkChain` requires the user to specify either the `qpoints` or `qpoints_distance` input.
+This means that work chains which wrap the `PhBaseWorkChain` but provide the q-points on the fly will have to disable this validation by setting the corresponding validator to `None` in the input spec.
+To avoid this, we have the validator check if one of the `qpoints` or `qpoints_distance` ports are still present in the port namespace.
+If not, the validation is skipped.
+
+Higher-level work chains that wrap the `PhBaseWorkChain` can then simply exclude these ports when exposing the inputs:
+
+```python
+    class WrapPhBaseWorkChain(WorkChain):
+        """Example work chain that wraps a ``PhBaseWorkChain`` excluding q-points inputs."""
+
+        @classmethod
+        def define(cls, spec):
+            super().define(spec)
+            spec.expose_inputs(PhBaseWorkChain, exclude=('qpoints', 'qpoints_distance'))
+```
+
+### 👌 Improvements
+
+* `PhBaseWorkChain`: skip q-points validation if ports are excluded [[32536e8](https://github.com/aiidateam/aiida-quantumespresso/commit/32536e85abd6de30cd8f9a07124996ce8cd0760a)]
+
+### 🐛 Bug fixes
+
+* `PhBaseWorkChain`: fix `set_qpoints` step [[c353cc2](https://github.com/aiidateam/aiida-quantumespresso/commit/c353cc2e4104352ef9b5490adb53a60da47f293d)]
+
+### 📚 Documentation
+
+* `CHANGELOG.md`: improve release notes for `v4.5.0` [[b659625](https://github.com/aiidateam/aiida-quantumespresso/commit/b65962565f300fbda643ea43d18d01329c4a85ff)]
+
 ## v4.5.0
 
+Besides several bug fixes and documentation improvements, this minor release introduces some changes to the `PhBaseWorkChain` and how it is used.
+Most importantly, q-points are no longer defined directly on the `PhCalculation` in the `ph` namespace, but as a top-level input on the `PhBaseWorkChain`, either as a `KpointsData` node (see [b9c7517](https://github.com/aiidateam/aiida-quantumespresso/commit/b9c7517744e645a93d4afc9b1999881fc39a0e46)):
+
+```python
+ph_base_builder = PhBaseWorkChain.get_builder()
+
+qpoints = orm.KpointsData()
+qpoints.set_kpoints_mesh([2, 2, 2])
+
+ph_base_builder.qpoints = qpoints
+```
+
+Or a `qpoints_distance`, which defines a linear density that can be used in combination with the structure to generate a mesh on the fly:
+
+```python
+ph_base_builder = PhBaseWorkChain.get_builder()
+
+ph_base_builder.qpoints_distance = orm.Float(0.3)
+```
+
+The protocols are also updated to use the new `qpoints_distance` input, with some reasonable default for the various options.
+
+| ⚠️ **Important**: While the current defaults are reasonable, they by no means represent a rigorously tested protocol that guarantees a certain level of convergence. Be sure to run your own tests! |
+|---|
+
+The `get_builder_from_protocol()` method of the `PhBaseWorkChain` now also recognises the `electronic_type` input argument.
+For example, when running an insulator, one can obtain a fully populated builder via:
+
+```python
+from aiida_quantumespresso.common.types import ElectronicType
+
+builder = PhBaseWorkChain.get_builder_from_protocol(
+    code=ph_code,
+    parent_folder=pw_remote_folder,
+    electronic_type=ElectronicType.INSULATOR
+)
+```
+
+which will set `INPUTPH.epsil` to `True` in the `ph.x` input file.
+
+This release also bumps the default SSSP version in the protocols up to v1.3 (still using the PBEsol functional).
+In case you have not installed this newer version, you can do so with `aiida-pseudo` (version `>=1.1.0`):
+
+```console
+aiida-pseudo install sssp -v 1.3 -x PBEsol
+```
+
+### ‼️ Breaking changes
+
+- `PhBaseWorkChain`: allow generation of q-point mesh via `qpoints_distance` [[b9c7517]](https://github.com/aiidateam/aiida-quantumespresso/commit/b9c7517744e645a93d4afc9b1999881fc39a0e46)
+
 ### ✨ New features
+
 - `PwBaseWorkChain`: new handler for BFGS history failure [[0224f8a]](https://github.com/aiidateam/aiida-quantumespresso/commit/0224f8a4cf8122a916e2b2c5be11f3a8a811f740)
-- `PhBaseWorkChain`: allow generation of q-point mesh via `qpoints_distance` [[b9c7517]](https://github.com/aiidateam/aiida-quantumespresso/commit/b9c7517744e645a93d4afc9b1999881fc39a0e46)
 - Support calculating the XPS spectra of the atoms specific by indices [[fc1a940]](https://github.com/aiidateam/aiida-quantumespresso/commit/fc1a940d4a60f22b42ec0a069a06436c0c9ae0f5)
 
+### 👌 Improvements
+
+- Improve `PhBaseWorkChain` overrides/protocol [[39287e0]](https://github.com/aiidateam/aiida-quantumespresso/commit/39287e03cb6bbf1915662685a5c441e9c7c36030)
+- `PwBaseWorkChain`: Remove disabling of resource validation [[d4e6681]](https://github.com/aiidateam/aiida-quantumespresso/commit/d4e668195d369e360bfb1f06611049a940640843)
+- Protocols: Bump default SSSP version to 1.3 [[49d503d]](https://github.com/aiidateam/aiida-quantumespresso/commit/49d503d8b2a0c09dd2b38fecb73b28c82e930822)
+
 ### 🐛 Bug fixes
+
 - `PdosWorkChain`: Fix constrained magnetization case [[a68e1e1]](https://github.com/aiidateam/aiida-quantumespresso/commit/a68e1e15c11f6ad4461921145c648d75c49ff26c)
 - Fix missing `max_iterations` in overrides [[9061ea5]](https://github.com/aiidateam/aiida-quantumespresso/commit/9061ea5df65e4fe95f77309d9abb5a3c7f64bb9f)
 - `OpenGridCalculation`: Add the `output_parameters` output to the spec [[5f0e095]](https://github.com/aiidateam/aiida-quantumespresso/commit/5f0e095647d6529c295002aa15e48ff647111ab7)
 - `Q2rCalculation`: Add the `output_parameters` output to the spec [[7a303f9]](https://github.com/aiidateam/aiida-quantumespresso/commit/7a303f9e10ff6dd9eae652b4f2d8ad2c482022d6)
-- `PwBaseWorkChain`: Pop `starting_magnetization if `tot_magnetization` is defined [[2adf033]](https://github.com/aiidateam/aiida-quantumespresso/commit/2adf0335291b452d41ee9282ef13ed21f47cebc8)
+- `PwBaseWorkChain`: Pop `starting_magnetization` if `tot_magnetization` is defined [[2adf033]](https://github.com/aiidateam/aiida-quantumespresso/commit/2adf0335291b452d41ee9282ef13ed21f47cebc8)
 - `PwCalculation`: Fix calling input validation of base class [[17e173f]](https://github.com/aiidateam/aiida-quantumespresso/commit/17e173f11c75142755bc9f3c9a71160d5aba778c)
 
-### 👌 Improvements
-- Improve `PhBaseWorkChain` overrides/protocol [[39287e0]](https://github.com/aiidateam/aiida-quantumespresso/commit/39287e03cb6bbf1915662685a5c441e9c7c36030)
-- `PwBaseWorkChain`: Remove disabling of resource validation [[d4e6681]](https://github.com/aiidateam/aiida-quantumespresso/commit/d4e668195d369e360bfb1f06611049a940640843)
-- Protocols: Bump default SSSP version to 1.3 [[49d503d]](https://github.com/aiidateam/aiida-quantumespresso/commit/49d503d8b2a0c09dd2b38fecb73b28c82e930822)
+### 📚 Documentation
+
+- Remove `aiida.manage.configuration.load_documentation_profile` [[f1d547c]](https://github.com/aiidateam/aiida-quantumespresso/commit/f1d547c28b35241a53703b0790cf5dac70455060)
+- Address warning from `pydata-sphinx-theme` [[74bbaa2]](https://github.com/aiidateam/aiida-quantumespresso/commit/74bbaa22b383b3323fcc3d41ad5b82fa89895c92)
+- Docs: Fix broken build by updating `sphinx-autoapi~=3.0` [[80e550e]](https://github.com/aiidateam/aiida-quantumespresso/commit/80e550e9e4d831b620f61fb93c88fcc0778a467d)
+- Docs: Update QE compatibility matrix in README.md [[5db3b28]](https://github.com/aiidateam/aiida-quantumespresso/commit/5db3b28ca5e067e63e59fdfdf6be8362efc7d223)
+
+### 🔧 Maintenance
+
+- Address various deprecation warnings from `aiida-core` [[f133b9a]](https://github.com/aiidateam/aiida-quantumespresso/commit/f133b9ab8c87c122e0edff2d27bad54d5d834681)
 
 ### ⬆️ Update dependencies
 - Dependencies: Update `pydantic~=2.4` [[740e0be]](https://github.com/aiidateam/aiida-quantumespresso/commit/740e0bec0e68b3229367b9f10b181a925616c08b)
 - Dependencies: Update `xmlschema~=2.0` [[bec6dd6]](https://github.com/aiidateam/aiida-quantumespresso/commit/bec6dd6b56b4cd3bbed3f3ab8fb97c7f7bdc0214)
 
-### 📚 Documentation
-- Devops: Address various deprecation warnings from `aiida-core` [[f133b9a]](https://github.com/aiidateam/aiida-quantumespresso/commit/f133b9ab8c87c122e0edff2d27bad54d5d834681)
-- Docs: Remove `aiida.manage.configuration.load_documentation_profile` [[f1d547c]](https://github.com/aiidateam/aiida-quantumespresso/commit/f1d547c28b35241a53703b0790cf5dac70455060)
-- Docs: Address warning from `pydata-sphinx-theme` [[74bbaa2]](https://github.com/aiidateam/aiida-quantumespresso/commit/74bbaa22b383b3323fcc3d41ad5b82fa89895c92)
-- Docs: Fix broken build by updating `sphinx-autoapi~=3.0` [[80e550e]](https://github.com/aiidateam/aiida-quantumespresso/commit/80e550e9e4d831b620f61fb93c88fcc0778a467d)
-- Docs: Update QE compatibility matrix in README.md [[5db3b28]](https://github.com/aiidateam/aiida-quantumespresso/commit/5db3b28ca5e067e63e59fdfdf6be8362efc7d223)
-- Tests: Revert change to `fixture_code` [[e9ce7a0]](https://github.com/aiidateam/aiida-quantumespresso/commit/e9ce7a069cf1011f829143563730750a9b9fc637)
+### 🧪 Tests
+
+- Revert change to `fixture_code` [[e9ce7a0]](https://github.com/aiidateam/aiida-quantumespresso/commit/e9ce7a069cf1011f829143563730750a9b9fc637)
 
 
 ## v4.4.0

src/aiida_quantumespresso/__init__.py

@@ -1,3 +1,3 @@
 # -*- coding: utf-8 -*-
 """The official AiiDA plugin for Quantum ESPRESSO."""
-__version__ = '4.5.0'
+__version__ = '4.5.1'

src/aiida_quantumespresso/calculations/functions/seekpath_structure_analysis.py

@@ -3,6 +3,8 @@
 from aiida.engine import calcfunction
 from aiida.orm import Data
 
+from aiida_quantumespresso.data.hubbard_structure import HubbardStructureData
+
 
 @calcfunction
 def seekpath_structure_analysis(structure, **kwargs):
@@ -28,4 +30,31 @@ def seekpath_structure_analysis(structure, **kwargs):
     # All keyword arugments should be `Data` node instances of base type and so should have the `.value` attribute
     unwrapped_kwargs = {key: node.value for key, node in kwargs.items() if isinstance(node, Data)}
 
-    return get_explicit_kpoints_path(structure, **unwrapped_kwargs)
+    result = get_explicit_kpoints_path(structure, **unwrapped_kwargs)
+
+    if isinstance(structure, HubbardStructureData):
+        result['primitive_structure'] = update_structure_with_hubbard(result['primitive_structure'], structure)
+        result['conv_structure'] = update_structure_with_hubbard(result['conv_structure'], structure)
+
+    return result
+
+
+def update_structure_with_hubbard(structure, orig_structure):
+    """Update the structure based on Hubbard parameters if the input structure is a HubbardStructureData."""
+    from aiida_quantumespresso.utils.hubbard import is_intersite_hubbard
+
+    hubbard_structure = HubbardStructureData.from_structure(structure)
+
+    if is_intersite_hubbard(orig_structure.hubbard):
+        raise NotImplementedError('Intersite Hubbard parameters are not yet supported.')
+
+    for parameter in orig_structure.hubbard.parameters:
+        hubbard_structure.initialize_onsites_hubbard(
+            atom_name=orig_structure.sites[parameter.atom_index].kind_name,
+            atom_manifold=parameter.atom_manifold,
+            value=parameter.value,
+            hubbard_type=parameter.hubbard_type,
+            use_kinds=True,
+        )
+
+    return hubbard_structure

src/aiida_quantumespresso/data/hubbard_structure.py

@@ -5,6 +5,7 @@
 
 from aiida.orm import StructureData
 import numpy as np
+from pymatgen.core import Lattice, PeriodicSite
 
 from aiida_quantumespresso.common.hubbard import Hubbard, HubbardParameters
 
@@ -58,6 +59,7 @@ def hubbard(self) -> Hubbard:
 
         :returns: a :class:`~aiida_quantumespresso.common.hubbard.Hubbard` instance.
         """
+        # pylint: disable=not-context-manager
         with self.base.repository.open(self._hubbard_filename, mode='rb') as handle:
             return Hubbard.model_validate_json(json.load(handle))
 
@@ -109,8 +111,19 @@ def append_hubbard_parameter(
         :param hubbard_type: hubbard type (U, V, J, ...), defaults to 'Ueff'
             (see :class:`~aiida_quantumespresso.common.hubbard.Hubbard` for full allowed values)
         """
-        pymat = self.get_pymatgen_structure()
-        sites = pymat.sites
+        sites = [
+            PeriodicSite(
+                species=site.species,
+                coords=site.coords,
+                lattice=Lattice(self.cell, pbc=self.pbc),
+                coords_are_cartesian=True
+            ) for site in self.get_pymatgen().sites
+        ]
+
+        if any((atom_index > len(sites) - 1, neighbour_index > len(sites) - 1)):
+            raise ValueError(
+                'atom_index and neighbour_index must be within the range of the number of sites in the structure'
+            )
 
         if translation is None:
             _, translation = sites[atom_index].distance_and_image(sites[neighbour_index])

src/aiida_quantumespresso/workflows/ph/base.py

@@ -69,9 +69,11 @@ def define(cls, spec):
         # yapf: enable
 
     @classmethod
-    def validate_inputs(cls, inputs, ctx=None):  # pylint: disable=unused-argument
+    def validate_inputs(cls, value, port_namespace):  # pylint: disable=unused-argument
         """Validate the top level namespace."""
-        if 'qpoints_distance' not in inputs and 'qpoints' not in inputs:
+
+        if (('qpoints_distance' in port_namespace or 'qpoints' in port_namespace) and
+            'qpoints_distance' not in value and 'qpoints' not in value):
             return 'Neither `qpoints` nor `qpoints_distance` were specified.'
 
     @classmethod
@@ -177,27 +179,26 @@ def set_qpoints(self):
         the case of the latter, the `KpointsData` will be constructed for the input `StructureData`
         from the parent_folder using the `create_kpoints_from_distance` calculation function.
         """
-
         try:
             qpoints = self.inputs.qpoints
         except AttributeError:
 
             try:
-                structure = self.ctx.inputs.ph.parent_folder.creator.output.output_structure
+                structure = self.ctx.inputs.parent_folder.creator.output.output_structure
             except AttributeError:
-                structure = self.ctx.inputs.ph.parent_folder.creator.inputs.structure
+                structure = self.ctx.inputs.parent_folder.creator.inputs.structure
 
             inputs = {
                 'structure': structure,
                 'distance': self.inputs.qpoints_distance,
-                'force_parity': self.inputs.qpoints_force_parity,
+                'force_parity': self.inputs.get('qpoints_force_parity', orm.Bool(False)),
                 'metadata': {
                     'call_link_label': 'create_qpoints_from_distance'
                 }
             }
             qpoints = create_kpoints_from_distance(**inputs)
 
-        self.ctx.inputs.ph['qpoints'] = qpoints
+        self.ctx.inputs['qpoints'] = qpoints
 
     def set_max_seconds(self, max_wallclock_seconds: None):
         """Set the `max_seconds` to a fraction of `max_wallclock_seconds` option to prevent out-of-walltime problems.

tests/calculations/functions/test_seekpath_analysis.py

@@ -0,0 +1,69 @@
+# -*- coding: utf-8 -*-
+"""Tests for the `seekpath_structure_analysis` function for HubbbardStructureData."""
+import pytest
+
+from aiida_quantumespresso.calculations.functions.seekpath_structure_analysis import seekpath_structure_analysis
+from aiida_quantumespresso.data.hubbard_structure import HubbardStructureData
+
+
+# pylint: disable=W0621
+@pytest.mark.usefixtures('aiida_profile')
+def test_seekpath_analysis(data_regression):
+    """Test the `seekpath_structure_analysis` calculation function for HubbardStructureData."""
+    cell = [[5.43, 0.0, 0.0], [0.0, 5.43, 0.0], [0.0, 0.0, 5.43]]
+    sites = (
+        ('Co', 'Co0', (0.0, 0.0, 0.0)),
+        ('Co', 'Co0', (0.0, 2.715, 2.715)),
+        ('Co', 'Co1', (2.715, 0.0, 2.715)),
+        ('Co', 'Co1', (2.715, 2.715, 0.0)),
+    )
+    orig_structure = HubbardStructureData(cell=cell, sites=sites)
+
+    for hubbard_parameter in [
+        ('Co0', '3d', 4.0, 'Ueff', True),
+        ('Co1', '3d', 3.0, 'U', True),
+    ]:
+        orig_structure.initialize_onsites_hubbard(*hubbard_parameter)
+
+    result = seekpath_structure_analysis(orig_structure)
+
+    prim_structure = result['primitive_structure']
+    conv_structure = result['conv_structure']
+
+    assert isinstance(prim_structure, HubbardStructureData), 'Primitive structure should be a HubbardStructureData'
+    assert isinstance(conv_structure, HubbardStructureData), 'Conventional structure should be a HubbardStructureData'
+
+    assert prim_structure.hubbard.parameters != orig_structure.hubbard.parameters, \
+        'Primitive parameters should be different'
+    assert len(prim_structure.hubbard.parameters) == len(orig_structure.hubbard.parameters), \
+        'Primitive parameters should have the same length as original parameters'
+    assert all(
+        prim_param.atom_manifold == orig_param.atom_manifold
+        for prim_param, orig_param in zip(prim_structure.hubbard.parameters, orig_structure.hubbard.parameters)
+    ), 'Primitive cell parameter atom manifolds should match the original'
+
+    data_regression.check({
+        'primitive': {
+            'cell': prim_structure.cell,
+            'kinds': prim_structure.get_site_kindnames(),
+            'positions': [site.position for site in prim_structure.sites],
+            'hubbard': prim_structure.hubbard.to_list(),
+        },
+        'conventional': {
+            'cell': conv_structure.cell,
+            'kinds': conv_structure.get_site_kindnames(),
+            'positions': [site.position for site in conv_structure.sites],
+            'hubbard': conv_structure.hubbard.to_list(),
+        }
+    })
+
+
+# pylint: disable=W0621
+@pytest.mark.usefixtures('aiida_profile')
+def test_seekpath_analysis_intersite(generate_structure):
+    """Test that the `seekpath_structure_analysis` with intersite hubbard corrections fails."""
+    orig_structure = HubbardStructureData.from_structure(generate_structure('silicon-kinds'))
+    orig_structure.initialize_intersites_hubbard('Si0', '2p', 'Si1', '2p', 4.0, 'V', True)
+
+    with pytest.raises(NotImplementedError, match='Intersite Hubbard parameters are not yet supported.'):
+        seekpath_structure_analysis(orig_structure)