aiidateam / aiida-workgraph

Efficiently design and manage flexible workflows with AiiDA, featuring an interactive GUI, checkpoints, provenance tracking, and remote execution capabilities.
https://aiida-workgraph.readthedocs.io/en/latest/
MIT License
7 stars 5 forks source link

Misleading error message from serializer when passing `StructureData` instead of `Atoms` #163

Open GeigerJ2 opened 1 month ago

GeigerJ2 commented 1 month ago

I just tried running the eos WorkGraph from workgraph_collections and ended up passing a StructureData where an Atoms object should have been passed. The traceback I got (see below) was a bit misleading, and led me to think there was something wrong with my other inputs, e.g., the metadata dictionary (that is, until I printed the actual offending data object in the ValueError). As StructureData doesn't have a .value property, the exception was triggered and assumed it wasn't an AiiDA data type. Maybe the check can be further refined here, or at least the object added in the exception for easier debugging.

ValueError                                Traceback (most recent call last)
Cell In[53], line 28
     14 wg = eos_workgraph(
     15     # atoms=atoms,
     16     atoms=structure,
   (...)
     24     # metadata=metadata,
     25 )
     26 # ------------------------- Submit the calculation -------------------
     27 # wg.run()
---> 28 wg.submit(wait=True, timeout=200)

File ~/aiida_projects/workgraph-dev/git-repos/aiida-workgraph/aiida_workgraph/workgraph.py:133, in WorkGraph.submit(self, inputs, wait, timeout, metadata)
    130         self.tasks[name].set(input)
    132 # save the workgraph to the process node
--> 133 self.save(metadata=metadata)
    134 if self.process.process_state.value.upper() not in [\"CREATED\"]:
    135     raise ValueError(f\"Process {self.process.pk} has already been submitted.\")

File ~/aiida_projects/workgraph-dev/git-repos/aiida-workgraph/aiida_workgraph/workgraph.py:152, in WorkGraph.save(self, metadata)
    149 from aiida.engine.utils import instantiate_process
    150 from aiida_workgraph.engine.workgraph import WorkGraphEngine
--> 152 inputs = self.prepare_inputs(metadata)
    153 if self.process is None:
    154     runner = manager.get_manager().get_runner()

File ~/aiida_projects/workgraph-dev/git-repos/aiida-workgraph/aiida_workgraph/workgraph.py:73, in WorkGraph.prepare_inputs(self, metadata)
     71 wgdata = self.to_dict()
     72 merge_properties(wgdata)
---> 73 serialize_pythonjob_properties(wgdata)
     74 metadata = metadata or {}
     75 inputs = {\"wg\": wgdata, \"metadata\": metadata}

File ~/aiida_projects/workgraph-dev/git-repos/aiida-workgraph/aiida_workgraph/utils/__init__.py:321, in serialize_pythonjob_properties(wgdata)
    315 # if value is not None, not {}
    316 if not (
    317     prop[\"value\"] is None
    318     or isinstance(prop[\"value\"], dict)
    319     and prop[\"value\"] == {}
    320 ):
--> 321     prop[\"value\"] = general_serializer(prop[\"value\"])

File ~/aiida_projects/workgraph-dev/git-repos/aiida-workgraph/aiida_workgraph/orm/serializer.py:75, in general_serializer(data, check_value)
     72 if isinstance(data, orm.Data):
     73     if check_value and not hasattr(data, \"value\"):
     74         # print()
---> 75         raise ValueError(\"Only AiiDA data Node with a value attribute is allowed.\
\", data)
     76     return data
     77 elif isinstance(data, common.extendeddicts.AttributeDict):
     78     # if the data is an AttributeDict, use it directly

ValueError: ('Only AiiDA data Node with a value attribute is allowed.\
', <StructureData: uuid: ce586a59-89cd-4f9c-8278-e475ce3a984c (unstored)>)"
### Tasks
superstar54 commented 1 month ago

@GeigerJ2. Thanks for reporting the issue. Could you fix it?

btw. There are two EOS examples for quantum espresso; one is using aside-quantumespresso, and another using an ASE espresso calculator (PythonJob).

GeigerJ2 commented 1 month ago

Hi @superstar54, sure, I'll see when I get around implementing a fix. Yes, I saw the one using the ASE espresso calculator, but wasn't aware of the one using aiida-quantumespresso. Could you point me to that, please? Or do you just mean the general EOS workchain? Ideally, I'd just run the pw.x executable via ShellJob, using a standard function to write the input (as outlined here), and then a minimal parser to get the results. The idea is to keep it as minimal as possible, with no AiiDA boilerplate, to get a good comparison for the ADIS project. At least, that's my idea, and I think with WorkGraph and aiida-shell we can achieve that.

superstar54 commented 1 month ago

using aiida-quantumespresso CalcJob:

Source code: https://github.com/superstar54/workgraph-collections/blob/main/workgraph_collections/qe/eos.py Example usage: https://workgraph-collections.readthedocs.io/en/latest/qe/eos.html

using ASE espresso calculation

Source code: https://github.com/superstar54/workgraph-collections/blob/main/workgraph_collections/ase/espresso/eos.py Example usage: https://workgraph-collections.readthedocs.io/en/latest/ase/espresso/eos.html

I think with WorkGraph and aiida-shell we can achieve that.

Yes

GeigerJ2 commented 1 month ago

Cheers, thanks for the pointers! <3

Now, ideally, we wouldn't rely on PwCalculation of aiida-quantumespresso and aiida-pseudo, but run pw.x directly via ShellJob so we can fit all the code snuggly in a small jupyter notebook (which is what I've been trying today).