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

ProcessFunction: Automatically serialize Python base type inputs #5688

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Oct 5, 2022

Fixes #5686

The to_aiida_type dispatcher is set as the serializer of the input ports of the ProcessFunction process class. This process class is generated on the fly for a function decorated by the calcfunction and workfunction decorators.

Through change, process functions can now be called with Python base types for their arguments, which will be automatically wrapped in the corresponding AiiDA data type.

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

@sphuber thanks! Really a quick and clear implementation. I am curious, what if the input argument is a complex datatype, which error it will raise?

@sphuber
Copy link
Contributor Author

sphuber commented Oct 5, 2022

@sphuber thanks! Really a quick and clear implementation. I am curious, what if the input argument is a complex datatype, which error it will raise?

If a type is passed that is not known by to_aiida_type nothing is serialized and the value itself is passed. It will then go to the normal type validation on the corresponding port and if the type is incorrect, an error is raised like normal. It would be like passing a StructureData to a port that would expect an Int.

unkcpz
unkcpz previously approved these changes Oct 5, 2022
Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

LGTM!🚀

@ltalirz
Copy link
Member

ltalirz commented Oct 7, 2022

This looks very useful to me; it will also make people wonder why they can't do the same with other processes ;-)

One comment: perhaps it makes sense to adapt some existing tests of work/calcfunctions, replacing AiiDA type inputs with python types, just to have an "integration test" as well.

@ltalirz
Copy link
Member

ltalirz commented Oct 7, 2022

No further review needed from my side.

Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

Yep all good cheers

@sphuber
Copy link
Contributor Author

sphuber commented Oct 7, 2022

This looks very useful to me; it will also make people wonder why they can't do the same with other processes ;-)

We could consider to set to_aiida_type as the default serializer for all newly created InputPorts. This would achieve the desired effect. Users will just have to realize that if they replace the serializer, the functionality is lost. But usually that is anyway when you are specifying an input port with a valid type that is not a Python base type, so I doubt this would matter much.

The `to_aiida_type` dispatcher is set as the `serializer` of the input
ports of the `ProcessFunction` process class. This process class is
generated on the fly for a function decorated by the `calcfunction` and
`workfunction` decorators.

Through change, process functions can now be called with Python base
types for their arguments, which will be automatically wrapped in the
corresponding AiiDA data type.
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.

Add automatic input serialization to ProcessFunctions
4 participants