KeyError: 'CONTROL'

[alinelena]

I am trying a test installation of aiida with qe espresson (7.0,7.1 soon 7.2) and I am getting the error above, full trace is below. (I did not have issues in the past)

the script I test with is also below.

calculation itself runs correctly

# -*- coding: utf-8 -*-
###############################################################################
"""Run simple DFT calculation."""

import os
import sys
import click

from aiida.common import NotExistent
from aiida.engine import run
from aiida.orm import Dict, KpointsData, StructureData, load_code, load_group
from ase.build import bulk


StructureData = DataFactory('structure')  # pylint: disable=invalid-name


def example_dft(qe_code):
    """Run simple DFT calculation."""

    thisdir = os.path.dirname(os.path.realpath(__file__))
    # Structure.
    structure = StructureData(ase=bulk('Si', 'fcc', 5.43))
    pseudo_family = load_group('SSSP/1.3/PBE/efficiency')
    pseudos = pseudo_family.get_pseudos(structure=structure)
    cutoff_wfc, cutoff_rho = pseudo_family.get_recommended_cutoffs(
    structure=structure,
    unit='Ry'
)
    parameters = Dict({
        'control': {
            'calculation': 'scf',
             'verbosity': 'high'
        },
        'system': {
            'ecutwfc': cutoff_wfc,  # wave function cutoff in Ry
            'ecutrho': cutoff_rho,  # density cutoff in Ry
        },
    })
    kpoints = KpointsData()
    kpoints.set_kpoints_mesh([4,4,4])

    # Construct process builder.
    builder = qe_code.get_builder()
    builder.structure = structure
    builder.code = qe_code
    builder.pseudos = pseudos
    builder.kpoints = kpoints
    builder.parameters = parameters
    builder.metadata.options.resources = {
        "num_machines": 1
    }
    builder.metadata.options.max_wallclock_seconds = 1 * 3 * 60

    print("Submitted calculation...")
    results, node = run.get_node(builder)

    print(f"node status: {node.exit_status()}")

@click.command('cli')
@click.argument('codelabel')
def cli(codelabel):
    """Click interface."""
    try:
        code = Code.get_from_string(codelabel)
    except NotExistent:
        print(f"The code '{codelabel}' does not exist.")
        sys.exit(1)
    example_dft(code)


if __name__ == '__main__':
    cli()  # pylint: disable=no-value-for-parameter

Submitted calculation...
Report: [244|PwCalculation|on_except]: Traceback (most recent call last):
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/plumpy/process_states.py", line 228, in execute
    result = self.run_fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida/engine/processes/calcjobs/calcjob.py", line 678, in parse
    exit_code_retrieved = self.parse_retrieved_output(retrieved_temporary_folder)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida/engine/processes/calcjobs/calcjob.py", line 789, in parse_retrieved_output
    exit_code = parser.parse(**parse_kwargs)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida_quantumespresso/parsers/pw.py", line 71, in parse
    structure = self.build_output_structure(parsed_structure)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida_quantumespresso/parsers/pw.py", line 431, in build_output_structure
    type_calc = self.node.inputs.parameters.get_dict()['CONTROL']['calculation']
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
KeyError: 'CONTROL'

Traceback (most recent call last):
  File "/home/drFaustroll/micromamba/envs/aiida/bin/verdi", line 10, in <module>
    sys.exit(verdi())
             ^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida/cmdline/utils/decorators.py", line 73, in wrapper
    return wrapped(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida/cmdline/commands/cmd_run.py", line 115, in run
    exec(compile(handle.read(), str(filepath), 'exec', dont_inherit=True), globals_dict)  # pylint: disable=exec-used
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "test_qe_block.py", line 73, in <module>
    cli()  # pylint: disable=no-value-for-parameter
    ^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "test_qe_block.py", line 69, in cli
    example_dft(code)
  File "test_qe_block.py", line 56, in example_dft
    results, node = run.get_node(builder)
                    ^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida/engine/launch.py", line 60, in run_get_node
    return runner.run_get_node(process, *args, **inputs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida/engine/runners.py", line 277, in run_get_node
    result, node = self._run(process, *args, **inputs)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida/engine/runners.py", line 249, in _run
    process_inited.execute()
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/plumpy/processes.py", line 86, in func_wrapper
    return func(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/plumpy/processes.py", line 1197, in execute
    return self.future().result()
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/plumpy/process_states.py", line 228, in execute
    result = self.run_fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida/engine/processes/calcjobs/calcjob.py", line 678, in parse
    exit_code_retrieved = self.parse_retrieved_output(retrieved_temporary_folder)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida/engine/processes/calcjobs/calcjob.py", line 789, in parse_retrieved_output
    exit_code = parser.parse(**parse_kwargs)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida_quantumespresso/parsers/pw.py", line 71, in parse
    structure = self.build_output_structure(parsed_structure)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/drFaustroll/micromamba/envs/aiida/lib/python3.11/site-packages/aiida_quantumespresso/parsers/pw.py", line 431, in build_output_structure
    type_calc = self.node.inputs.parameters.get_dict()['CONTROL']['calculation']
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
KeyError: 'CONTROL'

[sphuber]

Thanks for the report. Could you please try to run the exact same script, but change the top-level keys in the parameters dictionary to all-caps, i.e.:

    parameters = Dict({
        'CONTROL': {
            'calculation': 'scf',
             'verbosity': 'high'
        },
        'SYSTEM': {
            'ecutwfc': cutoff_wfc,  # wave function cutoff in Ry
            'ecutrho': cutoff_rho,  # density cutoff in Ry
        },
    })

Notice that control and system are now capitalized. I think it should then work. In the meantime, I will work on a fix.

[alinelena]

hi Sebastiaan, indeed that is the issue. odd that one went back to f77 mindset, previous version were case insensitive.

[sphuber]

I am a bit surprised that you mention that this used to work for you. I did some testing and this behavior goes back to at least v3.3.0. The problem is that the PwCalculation class automatically converts the top-level keys in the parameters input to all-caps, and so in your case doesn't complain. However, the PwParser does not do this and checks for the capitalized keys, and so if it is not there, will raise a KeyError.

I noted this problematic behavior already 5 years ago: https://github.com/aiidateam/aiida-quantumespresso/issues/121 It is somewhat surprising that a user hasn't encountered this before yet, or at least, hasn't reported it. @mbercx maybe for v5.0 we can finally bite the bullet and enforce a single consistent casing naming rule and stick with it?

[mbercx]

@sphuber I agree that we need to make sure the casing is consistent. Else anywhere you check an input parameter or setting (not just in PwParser, but also anywhere in a work chain, also in other packages) you have to first convert the dictionary into one where you know the casing. I also thought about trying some kind of case-insensitive dict, but that is probably a bad idea. It's much easier to just enforce the casing than having to deal with this everywhere in the code base (e.g. also inputs/overrides from the protocols - the more I think of it the more convinced I am ^^). And I think we can write a pretty clear error message to users so they can fix it before running a calculation with some validation.

One idea I was still toying with was to "fix" the casing using a serializer:

import warnings
from aiida.engine import WorkChain

from aiida_quantumespresso.calculations import _uppercase_dict as uppercase_dict

class MyWorkChain(WorkChain):
    @classmethod
    def define(cls, spec):
        super().define(spec)
        
        # Define the inputs
        spec.input(
            'input_parameters',
            valid_type=orm.Dict,
            required=True,
            serializer=cls.serialize_input_parameters,
        )
        
        # Define the outline of the work chain
        spec.outline(
            cls.report_input,

        )

    def report_input(self):
        self.report(f"Input parameters: {self.inputs.input_parameters.get_dict()}")

    @staticmethod
    def serialize_input_parameters(value):
        """Validate the input parameters."""
        input_parameters = value.get_dict()

        has_lowercase = any(any(char.islower() for char in key) for key in input_parameters.keys())

        if has_lowercase:
            warnings.warn("At least one top-level key has a lowercase character. Setting to uppercase.")

        return orm.Dict(dict=uppercase_dict(input_parameters, 'parameters'))

But I quickly reliased that serializers only act upon non-Data types, indeed:

https://github.com/aiidateam/aiida-core/blob/d3788adea220107bce3582d246bcc9674b5e1571/aiida/engine/processes/ports.py#L103-L123

Is there a feature for updating an input port that I'm now aware of? Would it make sense to allow this for the current serializer input?

My thinking is that we can enforce the casing on the fly, warning the user that we made these changes. At least @unkcpz seems to also think this is the way to go. ^^

EDIT: Also: I don't think we have to wait for v5.0 to enforce this, since running with lowercase will most likely not work anywhere?

[sphuber]

But I quickly reliased that serializers only act upon non-Data types, indeed: https://github.com/aiidateam/aiida-core/blob/d3788adea220107bce3582d246bcc9674b5e1571/aiida/engine/processes/ports.py#L103-L123 Is there a feature for updating an input port that I'm now aware of?

No, the serializers would be it.

Would it make sense to allow this for the current serializer input?

Perhaps. The request has been made before to make it act on non-Data inputs as well: https://github.com/aiidateam/aiida-core/issues/5342 My response there kind of applies here as well. The serializer can still be used, users simply have to pass plain dicts. This is actually even easier for the user as well. It is just that currently they are used to having to pass explicit Dict nodes. I think ideally all ports that accept Dict nodes automatically convert normal dicts such that users become used to this. Then you can add the logic you describe in the serializer.

One concrete downside of allowing Data nodes to also be modified by serializers is that this may lead to unexpected behavior (and somewhat a loss of "explicit" provenance). If you pass in a (stored) node, the serializer could now decide to change it and actually return another node. This modification would not be captured. Not a problem per se, but could be if the user doesn't expect it. Maybe as a "rule" it would be better therefore to say that serializers will never modify existing nodes. But I can see arguments made for both sides

[mbercx]

Thanks @sphuber, you make some valid points. I agree that adapting the serializer would muddy the concept of what a serializer is supposed to do and potentially open a Pandora's box of unexpected behaviour. I'd like to think I'd handle this power responsibly (warn the user, perhaps raise an error if the node is stored etc) but who knows what those other naughty plugin developers will do. ^^ In the end I don't think it's worth adapting the serializer for this use case.

Instead let's:

• [ ] Add serializers where appropriate.
• [ ] Adapt the parameters and settings serializer to adapt casing on the fly. To be discussed in https://github.com/aiidateam/aiida-quantumespresso/issues/121. Warn the user if the casing is changed.
• [ ] Add validators to enforce the established casing conventions.

Side note: Part of me likes the idea of having serializers and users just passing in regular Python types (or e.g. a pymatgen Structure). Seems more user-friendly. Part of me also worries that users might get less used to the concept of nodes as inputs and outputs and then be confused again after obtaining the outputs, but that's life. ^^

[sphuber]

Add serializers where appropriate.

The idea has been floated to add the to_aiida_type serializer` as the default serializer for input ports. This might be interesting to look at instead of starting to manually add it to all ports in our process specs.

Seems more user-friendly. Part of me also worries that users might get less used to the concept of nodes as inputs and outputs and then be confused again after obtaining the outputs, but that's life. ^^

This was the very reason that we originally decided against this. Also for not adding this automatic serialization to process functions. However, as of v2.3 we actually added the to_aiida_type as the default serializer for process function arguments. Maybe its best to actually go all the way and apply this to all processes.

Add validators to enforce the established casing conventions.

And what are those conventions? 😄 Upper- or lowercase?