Skip to content

Commit

Permalink
CLI: block family cutoffs set for established families
Browse files Browse the repository at this point in the history
One of the motivations of having dedicated and automated CLI commands to
install established pseudpotential families like the SSSP or the
Pseudo-dojo normconserving pseudopotentials is to try and prevent that
the pseudopotentials or recommended cutoffs deviate from the official
ones. However, the `family cutoffs set` command currently allows the
user to change the recommended cutoffs easily through the CLI.

In order to prevent this, we block usage of the command for `SsspFamily`
and `PseudoDojoFamily`'s, by adding an `exclude` input argument to the
`PseudoPotentialFamilyParam`, where the entry points of the classes that
should be disallowed can be provided. This is then used to adapt the
option decorator for the `PSEUDO_POTENTIAL_FAMILY` input argument of
`family cutoffs set`.

Note that the user _can_ still install e.g. the SSSP with their own
recommended cutoffs using the `install family` command as a
`CutoffsPseudoPotentialFamily`. However, the `SsspFamily` class is
reserved for pseudopotentials installed with the automated install
command.
  • Loading branch information
mbercx committed Oct 10, 2022
1 parent aa819c9 commit e22b5e7
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 28 deletions.
7 changes: 6 additions & 1 deletion docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
# Add any Sphinx extension module names here, as strings. They can be
# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
# ones.
extensions = ['sphinx_copybutton', 'autoapi.extension', 'sphinx_click']
extensions = ['sphinx_copybutton', 'autoapi.extension', 'sphinx_click', 'sphinx.ext.intersphinx']

# Settings for the `sphinx_copybutton` extension
copybutton_selector = 'div:not(.no-copy)>div.highlight pre'
Expand All @@ -31,6 +31,11 @@
autoapi_dirs = ['../../src/aiida_pseudo']
autoapi_ignore = ['*cli*']

# Settings for the `sphinx.ext.intersphinx` extension
intersphinx_mapping = {
'aiida': ('http://aiida_core.readthedocs.io/en/latest/', None),
}

# Add any paths that contain templates here, relative to this directory.
templates_path = ['_templates']

Expand Down
40 changes: 26 additions & 14 deletions docs/source/design.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ The two concepts are further explained below, especially focusing on how they ar
Pseudopotentials
================

Pseudopotentials are implemented as `data plugins <https://aiida-core.readthedocs.io/en/latest/topics/data_types.html#creating-a-data-plugin>`_, and the base class is ``PseudoPotentialData``.
Pseudopotentials are implemented as `data plugins <https://aiida-core.readthedocs.io/en/latest/topics/data_types.html#creating-a-data-plugin>`_, and the base class is :py:class:`~aiida_pseudo.data.pseudo.PseudoPotentialData`.
Pseudopotentials are assumed to be defined by a single file on disk and represent a single chemical element.
As such, each pseudopotential node, *has* to have two attributes: the md5 of the file that it represents and the symbol of chemical element that the pseudopotential describes.
The latter follows IUPAC naming conventions as used in the module ``aiida.common.constants.elements`` of ``aiida-core``.
The latter follows IUPAC naming conventions defined as ``elements`` in the module ``aiida.common.constants`` of ``aiida-core``.

The ``PseudoPotentialData`` functions as a base class and does not represent an actual existing pseudopotential format.
It *is* possible to create a family of ``PseudoPotentialData`` nodes, but since the element cannot be parsed from the file content itself, it will have to be done from the *filename*, so the filename *has* to have the format ``CHEMICAL_SYMBOL.EXTENSION``, for example ``Ar.pseudo``.
Since the ``PseudoPotentialData`` class does not have any features tailored to specific formats, as do the classes which inherit from it (e.g. ``UpfData`` for UPF-formatted pseudopotentials), this class is mostly useful for development.
:py:class:`~aiida_pseudo.data.pseudo.PseudoPotentialData` functions as a base class and does not represent an actual existing pseudopotential format.
It *is* possible to create a family of :py:class:`~aiida_pseudo.data.pseudo.PseudoPotentialData` nodes, but since the element cannot be parsed from the file content itself, it will have to be done from the *filename*, so the filename *has* to have the format ``CHEMICAL_SYMBOL.EXTENSION``, for example ``Ar.pseudo``.
Since the :py:class:`~aiida_pseudo.data.pseudo.PseudoPotentialData` class does not have any features tailored to specific formats, as do the classes which inherit from it (e.g. :py:class:`~aiida_pseudo.data.pseudo.UpfData` for UPF-formatted pseudopotentials), this class is mostly useful for development.

The plugin comes with a variety of pseudopotential subtypes that represent various common pseudopotential formats, such as UPF, PSF, PSML and PSP8.
The corresponding data plugins implement how the element and other data are parsed from a pseudopotential file with such a format.
Expand Down Expand Up @@ -52,11 +52,11 @@ Note that if more than one pseudopotential in the database is matched, a random
Families
========

Having loose pseudopotentials floating around is not very practical, so groups of related pseudopotentials can be organized into "families", which are implemented as group plugins with the base class ``PseudoPotentialFamily``.
Having loose pseudopotentials floating around is not very practical, so groups of related pseudopotentials can be organized into "families", which are implemented as group plugins with the base class :py:class:`~aiida_pseudo.groups.family.PseudoPotentialFamily`.
A family class can in principle support many pseudopotential formats, however, a family instance can only contain pseudopotentials of a single format.
For example, the ``PseudoPotentialFamily`` class supports all of the pseudopotential formats that are supported by this plugin.
For example, the :py:class:`~aiida_pseudo.groups.family.PseudoPotentialFamily` class supports all of the pseudopotential formats that are supported by this plugin.
However, any instance can only contain pseudopotentials of the same format (e.g. *all* UPF or *all* PSP8, not a mixture).
In contrast, the ``SsspFamily`` only supports the ``UpfData`` format.
In contrast, the :py:class:`~aiida_pseudo.groups.family.SsspFamily` only supports the :py:class:`~aiida_pseudo.data.pseudo.UpfData` format.

A pseudopotential family can be constructed manually, by first constructing the class instance and then adding pseudopotential data nodes to it:

Expand All @@ -77,7 +77,7 @@ A pseudopotential family can be constructed manually, by first constructing the
family = PseudoPotentialFamily(label='pseudos/upf').store()
family.append(pseudos)
Note that as with any ``Group``, it has to be stored before nodes can be added.
Note that as with any :py:class:`~aiida.orm.Group`, it has to be stored before nodes can be added.
If you have a folder on disk that contains various pseudopotentials for different elements, there is an even easier way to create the family automatically:

.. code-block:: python
Expand All @@ -90,17 +90,29 @@ If you have a folder on disk that contains various pseudopotentials for differen
family = PseudoPotentialFamily('path/to/pseudos', 'pseudos/upf', pseudo_type=UpfData)
The plugin is not able to reliably deduce the format of the pseudopotentials contained in the folder, so one should indicate what data type to use with the ``pseudo_type`` argument.
The exception is when the family class only supports a single pseudo type, such as for the ``SsspFamily``, in which case that type will automatically be selected.
The exception is when the family class only supports a single pseudo type, such as for the :py:class:`~aiida_pseudo.groups.family.SsspFamily`, in which case that type will automatically be selected.
Subclasses of supported pseudo types are also accepted.
For example, the base class ``PseudoPotentialFamily`` supports pseudopotentials of the ``PseudoPotentialData`` type.
Because all more specific pseudopotential types are subclasses of ``PseudoPotentialData``, the ``PseudoPotentialFamily`` class accepts all of them.
For example, the base class :py:class:`~aiida_pseudo.groups.family.PseudoPotentialFamily` supports pseudopotentials of the :py:class:`~aiida_pseudo.data.pseudo.PseudoPotentialData` type.
Because all more specific pseudopotential types are subclasses of py:class:`~aiida_pseudo.data.pseudo.PseudoPotentialData`, the :py:class:`~aiida_pseudo.groups.family.PseudoPotentialFamily` class accepts all of them.

Established families
--------------------

When it comes to pseudopotential families, ``aiida-pseudo`` makes a clear distinction between families that are *established* and those that are not.
A pseudopotential family is only considered to be *established* when it has a comprehensive set of rigorously tested pseudopotentials with convergence tests, both of which have been published and are publicly available.

Only a pseudopotential family that is *established* will receive support for automated installs with its own class (e.g. :py:class:`~aiida_pseudo.groups.family.SsspFamily`/:py:class:`~aiida_pseudo.groups.family.PseudoDojoFamily`) and command line interface (CLI) commands (e.g. ``install sssp``/``install pseudo-dojo``).
To make sure these families represent the official ones, they can only be installed with their supported CLI commands, and there are strict checks on the format of these files to make sure they correspond to the official ones.
Based on the same principle of preserving the integrity of these established pseudopotentials, the ``family cutoffs set`` command can not be used to set the recommended cutoffs of an established family.

If users want to install the a set of *non-established* pseudopotentials and configure their recommended cutoffs, they have to install them from the archive using ``install family`` as a :py:class:`~aiida_pseudo.groups.family.CutoffsPseudoPotentialFamily` as described in the :ref:`corresponding how-to section <how-to:install_archive>`.

Recommended cutoffs
===================

Certain pseudopotential family types, such as the ``SsspFamily``, provide recommended cutoff values for wave functions and charge density in plane-wave codes.
Certain pseudopotential family types, such as the :py:class:`~aiida_pseudo.groups.family.SsspFamily`, provide recommended cutoff values for wave functions and charge density in plane-wave codes.
These cutoffs can be defined in any unit supported by the |pint|_ package.
The recommended cutoffs for a set of elements or a ``StructureData`` can be retrieved from the family as follows:
The recommended cutoffs for a set of elements or a :py:class:`~aiida.orm.StructureData` can be retrieved from the family as follows:

.. code-block:: python
Expand Down
6 changes: 4 additions & 2 deletions src/aiida_pseudo/cli/family.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import click

from ..groups.mixins import RecommendedCutoffMixin
from .params import arguments, options
from .params import arguments, options, types
from .root import cmd_root


Expand Down Expand Up @@ -56,7 +56,9 @@ def cmd_family_cutoffs():


@cmd_family_cutoffs.command('set')
@arguments.PSEUDO_POTENTIAL_FAMILY()
@arguments.PSEUDO_POTENTIAL_FAMILY(
type=types.PseudoPotentialFamilyParam(exclude=('pseudo.family.sssp', 'pseudo.family.pseudo_dojo'))
)
@click.argument('cutoffs', type=click.File(mode='rb'))
@options.STRINGENCY(required=True)
@options.UNIT()
Expand Down
45 changes: 34 additions & 11 deletions src/aiida_pseudo/cli/params/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,17 @@


class PseudoPotentialTypeParam(click.ParamType):
"""Parameter type for `click` commands to define a subclass of `PseudoPotentialData`."""
"""Parameter type for ``click`` commands to define a subclass of ``PseudoPotentialData``."""

name = 'pseudo_type'

def convert(self, value, _, __):
"""Convert the entry point name to the corresponding class.
:param value: entry point name that should correspond to subclass of `PseudoPotentialData` data plugin
:return: the `PseudoPotentialData` subclass
:raises: `click.BadParameter` if the entry point cannot be loaded or is not subclass of `PseudoPotentialData`
:param value: entry point name that should correspond to subclass of ``PseudoPotentialData`` data plugin
:return: the ``PseudoPotentialData`` subclass
:raises: ``click.BadParameter`` if the entry point cannot be loaded or is not subclass of
``PseudoPotentialData``
"""
from aiida.common import exceptions
from aiida.plugins import DataFactory
Expand Down Expand Up @@ -54,13 +55,34 @@ def complete(self, _, incomplete):


class PseudoPotentialFamilyParam(GroupParamType):
"""Parameter type for `click` commands to define an instance of a `PseudoPotentialFamily`."""
"""Parameter type for ``click`` commands to define an instance of a ``PseudoPotentialFamily``."""

name = 'pseudo_family'

def __init__(self, exclude: typing.Optional[typing.List[str]] = None, **kwargs):
"""Construct the parameter.
:param exclude: an optional list of values that should be considered invalid and will raise ``BadParameter``.
"""
super().__init__(**kwargs)
self.exclude = exclude

def convert(self, value, param, ctx):
"""Convert the entry point name to the corresponding class.
:param value: entry point name that should correspond to subclass of ``PseudoPotentialFamily`` group plugin
:return: the ``PseudoPotentialFamily`` subclass
:raises: `click.BadParameter` if the entry point cannot be loaded or is not subclass of `PseudoPotentialFamily`
"""
family = super().convert(value, param, ctx)

if self.exclude and family.type_string in self.exclude:
self.fail(f'The value `{family}` is not allowed for this parameter.', param, ctx)
return family


class PseudoPotentialFamilyTypeParam(click.ParamType):
"""Parameter type for `click` commands to define a subclass of `PseudoPotentialFamily`."""
"""Parameter type for ``click`` commands to define a subclass of ``PseudoPotentialFamily``."""

name = 'pseudo_family_type'

Expand All @@ -75,9 +97,10 @@ def __init__(self, exclude: typing.Optional[typing.List[str]] = None, **kwargs):
def convert(self, value, _, __):
"""Convert the entry point name to the corresponding class.
:param value: entry point name that should correspond to subclass of `PseudoPotentialFamily` group plugin
:return: the `PseudoPotentialFamily` subclass
:raises: `click.BadParameter` if the entry point cannot be loaded or is not subclass of `PseudoPotentialFamily`
:param value: entry point name that should correspond to subclass of ``PseudoPotentialFamily`` group plugin
:return: the ``PseudoPotentialFamily`` subclass
:raises: `click.BadParameter` if the entry point cannot be loaded or is not subclass of `
`PseudoPotentialFamily``
"""
from aiida.common import exceptions
from aiida.plugins import GroupFactory
Expand Down Expand Up @@ -134,7 +157,7 @@ def convert(self, value, param, ctx) -> typing.Union[pathlib.Path, bytes]:


class UnitParamType(click.ParamType):
"""Parameter type to specify units from the `pint` library."""
"""Parameter type to specify units from the ``pint`` library."""

name = 'unit'

Expand All @@ -149,7 +172,7 @@ def __init__(self, quantity: typing.Optional[typing.List[str]] = None, **kwargs)
def convert(self, value, _, __):
"""Check if the provided unit is a valid unit for the defined quantity.
:raises: `click.BadParameter` if the provided unit is not valid for the quantity defined for this instance.
:raises: ``click.BadParameter`` if the provided unit is not valid for the quantity defined for this instance.
"""
if value not in U:
raise click.BadParameter(f'`{value}` is not a valid unit.')
Expand Down
27 changes: 27 additions & 0 deletions tests/cli/test_family.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
import pytest

from aiida_pseudo.cli.family import cmd_family_cutoffs_set, cmd_family_show
from aiida_pseudo.data.pseudo.upf import UpfData
from aiida_pseudo.groups.family import CutoffsPseudoPotentialFamily, PseudoPotentialFamily
from aiida_pseudo.groups.family.pseudo_dojo import PseudoDojoFamily
from aiida_pseudo.groups.family.sssp import SsspFamily


@pytest.mark.usefixtures('clear_db')
Expand Down Expand Up @@ -80,6 +83,30 @@ def test_family_cutoffs_set_unit(run_cli_command, get_pseudo_family, generate_cu
assert family.get_cutoffs_unit() == unit


@pytest.mark.parametrize(
'family_cls,label', [(SsspFamily, 'SSSP/1.1/PBE/efficiency'),
(PseudoDojoFamily, 'PseudoDojo/0.4/PBE/SR/standard/psp8')]
)
@pytest.mark.usefixtures('clear_db')
def test_family_cutoffs_set_established(
run_cli_command, get_pseudo_family, generate_cutoffs, tmp_path, family_cls, label
):
"""Test that `aiida-pseudo family cutoffs set` is blocked for established families."""
family = get_pseudo_family(
label=label,
cls=family_cls,
pseudo_type=UpfData,
)
stringency = 'normal'
cutoffs = generate_cutoffs(family)

filepath = tmp_path / 'cutoffs.json'
filepath.write_text(json.dumps(cutoffs))

result = run_cli_command(cmd_family_cutoffs_set, [family.label, str(filepath), '-s', stringency], raises=True)
assert f"Invalid value for 'FAMILY': The value `{family}` is not allowed for this parameter." in result.output


def test_family_show(clear_db, run_cli_command, get_pseudo_family):
"""Test the `aiida-pseudo show` command."""
family = get_pseudo_family()
Expand Down

0 comments on commit e22b5e7

Please sign in to comment.