Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed to get required imports for function #175

Closed
mbercx opened this issue Jul 17, 2024 · 2 comments · Fixed by #177
Closed

Failed to get required imports for function #175

mbercx opened this issue Jul 17, 2024 · 2 comments · Fixed by #177

Comments

@mbercx
Copy link
Member

mbercx commented Jul 17, 2024

While trying the nice new pythonjob decorator:

from aiida_workgraph import task
from functions import generate_structures

decorated_function = task.pythonjob()(generate_structures)

No error is raised, but the following message is printed:

Failed to get required imports for function generate_structures: type object 'list' has no attribute '_name'

The generate_structures function is the following:

def generate_structures(
        structure: Atoms,
        strain_lst: list
    ) -> list[Atoms]:

    structure_lst = []

    for strain in strain_lst:
        structure_strain = structure.copy()
        structure_strain.set_cell(
            structure_strain.cell * strain**(1/3), 
            scale_atoms=True
        )
        structure_lst.append(structure_strain)

    return structure_lst

I'm assuming the issue is the returned list? This issue also made me wonder how to deal with list outputs. The user here would probably want to have a list of linked AtomsData nodes, but I suppose that is not what will happen. Off the top of my head, I'm considering if the decorator function should have some kind of input argument that allows the user to specify that the list should be expanded as several linked output nodes.

@superstar54
Copy link
Member

superstar54 commented Jul 17, 2024

Failed to get required imports for function generate_structures: type object 'list' has no attribute '_name'

There is an error when the WorkGraph tries to inspect the type annotations, and it fails to inspect the list

strain_lst: list

This should be a bug. I will try to fix it this morning. For the moment, you can remove the type annotations, and try.

There are three possible solutions for the list outputs.

  • Use the function as it is so that PythonJob will Pickle the whole list and return a GeneralData (PickileData); you then pass this data into the next task. Note that the data provenance is lost.
  • Change the list to a dict, and set the output as Namespace, this will serialize the value of the dict. Thus each Atoms will be an AtomsData, and can used by the next task. The data provenance is kept.
def generate_structures(
        structure: Atoms,
        strain_lst: list
    ) -> list[Atoms]:

    structure_dict = {}

    for strain in strain_lst:
        structure_strain = structure.copy()
        structure_strain.set_cell(
            structure_strain.cell * strain**(1/3), 
            scale_atoms=True
        )
        structure_dict[f"strain_{strain}"] = structure_strain

    return structure_dict

decorated_function = task.pythonjob(outputs=[{"name": "structure_dict", "identifier": "Namespace"}])(generate_structures)
  • Support the list output, but convert to a dict by us with a default label index, and convert it back to a list when using it. This is not support yet, but we can add this.

What do you think?

superstar54 added a commit that referenced this issue Jul 17, 2024
The previous code has the assumption that all type hints which have an __origin__ attribute also possess an _name attribute. This is not true for certain built-in generic types like list, dict, etc. in Python, particularly when accessed via the __origin__ attribute, which is a legacy of Python's type hinting evolution.
@superstar54
Copy link
Member

Hi @mbercx , the get required imports bug is fixed in #177 .

However, the feature that allows users to return a list of AiiDA nodes is not yet. I opened a new issue #189 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants