Process.process_class raises on calcfunction closure

[ltalirz]

Some of the workchains in the aiida-lsmo plugin use "calcfunction closures", see e.g.
https://github.com/lsmo-epfl/aiida-lsmo/blob/0999ccec3e445cfd0dfd37a65ab013299a5f7d51/aiida_lsmo/workchains/isotherm.py#L283-L286

While this is perhaps not entirely clean (and could be avoided in this case by moving the schema to the toplevel outside the class), currently nothing prevents users from doing it and it seems to work fine

... unless you call Process.process_class. This property contains the following code path:

aiida-core/aiida/orm/nodes/process/process.py

Lines 124 to 133 in ec97fcf

	except ValueError:
	try:
	import importlib
	module_name, class_name = self.process_type.rsplit('.', 1)
	module = importlib.import_module(module_name)
	process_class = getattr(module, class_name)
	except (ValueError, ImportError):
	raise ValueError(
	f'could not load process class CalcJobNode<{self.pk}> given its `process_type`: {self.process_type}'
	)

In the example above, this code path fails with

2021-10-20 08:22:52 [52229 | REPORT]: [159960|SimAnnealingWorkChain|on_except]: Traceback (most recent call last):
  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/plugins/entry_point.py", line 136, in parse_entry_point_string
    group, name = entry_point_string.split(ENTRY_POINT_STRING_SEPARATOR)
ValueError: not enough values to unpack (expected 2, got 1)

 ...

  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/plugins/entry_point.py", line 138, in parse_entry_point_string
    raise ValueError('invalid entry_point_string format')
ValueError: invalid entry_point_string format
...
During handling of the above exception, another exception occurred:
  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/orm/nodes/node.py", line 1275, in <genexpr>
    return (node for node in nodes_identical if node.is_valid_cache)
  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/orm/nodes/process/process.py", line 486, in is_valid_cache
    process_class = self.process_class
  File "/home/miriam/anaconda3/envs/aiida/lib/python3.8/site-packages/aiida/orm/nodes/process/process.py", line 129, in process_class
    process_class = getattr(module, class_name)
AttributeError: module 'aiida_lsmo.workchains.isotherm' has no attribute 'get_valid_dict'

E.g. in the code above, the process_type is aiida_lsmo.workchains.isotherm.get_valid_dict, i.e. it does not include the WorkChain class.

This issue raises two questions for me:

1. Should calcfunction closures be allowed?
2. If yes, we may need to modify this code path.
   If not, it would be nice to be warned about this.

Any thoughts @sphuber ?

P.S. This issue was quite difficult to track down, since the code runs fine in the tests.
However, enabling caching results in a call to the .process_class property (only when the cache is already populated).

• AiiDA version: 1.6.5
• Python version: 3.8

[sphuber]

It makes sense that the problem only occurs when caching is enabled because the bug is in the Process.is_valid_cache property which only gets called when caching is enabled. The problem is that the statement does not catch the AttributeError that gets thrown. I will open a PR to fix this (also correcting the error messages that incorrectly refer to CalcJobNode.)

Then as to whether locally scoped calcfunction definitions are allowed: I think they are fine but the problem is that they cannot always be imported, which is the problem we are running into here. The thing I don't understand is how the calcfunction got its process_type. It seems to be aiida_lsmo.workchains.sim_annealing.get_valid_dict and that clearly is incorrect as get_valid_dict is defined locally in the setup method of the IsothermWorkChain workchain. Can you confirm that that is what is stored in the process_type attribute of one of those CalcFunctionNodes? Probably the code we use to build the full function module representation does not know how to properly deal with functions that are local to class methods.

[ltalirz]

Sorry, I copy-pasted things together from different trial runs - the get_valid_dict closure is present in both the isotherm and the sim_annealing workchains (now fixed in my comment above)

[ltalirz]

and thanks a lot for taking this on!

[ltalirz]

Can you confirm that that is what is stored in the process_type attribute of one of those CalcFunctionNodes? Probably the code we use to build the full function module representation does not know how to properly deal with functions that are local to class methods.

Yes.

Below is an example for another calcfunction (that is defined on the top-level of the module):

In [1]: load_node(166329)
Out[1]: <CalcFunctionNode: uuid: 3170f9bc-6bd4-4c19-8081-ae912c851212 (pk: 166329) (aiida_lsmo.workchains.isotherm.get_molecule_dict)>

In [2]: n=load_node(166329)

In [3]: n.process_type
Out[3]: 'aiida_lsmo.workchains.isotherm.get_molecule_dict'

For get_valid_dict I don't have them in the verdi process list but I printed the value of the process_type in the code path above and confirm that it is the one reported also in the error message.