Skip to content

Replace pythonjob task by normal task, drop usage of pickle_node #35

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

Merged
merged 7 commits into from
Apr 2, 2025

Conversation

superstar54
Copy link
Contributor

In my workstation, it took ~7 mins to run the QE workflow using AiiDA-WorkGraph, while other engines only take ~ 1 mins. The main reason is that the PythonJob task was used to auto-serialize inputs/outputs and preserve data provenance. However, PythonJob is designed to run the Python function on a remote computer. It uploads inputs to a remote folder and retrieves outputs to the local computer. In this project, it's overkill. In the latest aiida-workgraph, the normal keep provenance too. So this PR drops the usage of the PythonJob task.

Another reason is that, the pickled_node task is created for each data node, in this PR, when loading the workflow from a json file, the normal task can also serialize the inputs/outputs in an automatic way, so the pickle_node is not needed anymore. When writing the workflow using AiiDA, we prepare the AiiDA data nodes as inputs, so pickle_node is not needed either.

This PR reduces execution time from ~7 mins to ~ 1.5 mins.

This PR needs aiida-workgraph>=0.5.1.

@GeigerJ2
Copy link
Collaborator

GeigerJ2 commented Apr 2, 2025

Thanks for the nice changes, @superstar54! (both here, as well as in WG itself)
CI runs through now. Had to update the simple workflow to use the new approach, and currently, I'm still installing aiida-workgraph v0.5.1 via pip in the environment.yml file. We can wait with the merge until it's available on conda (thanks, @jan-janssen!), or merge it now already, I don't really have a strong opinion.

Copy link
Collaborator

@GeigerJ2 GeigerJ2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and to the 🌔 🚀

@jan-janssen jan-janssen merged commit 6f0dec1 into pythonworkflow:main Apr 2, 2025
4 checks passed
@jan-janssen jan-janssen mentioned this pull request Apr 2, 2025
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 this pull request may close these issues.

3 participants