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

Callbacks with parameters. #596

Merged
merged 117 commits into from
Mar 22, 2024
Merged

Conversation

erick-xanadu
Copy link
Contributor

@erick-xanadu erick-xanadu commented Mar 8, 2024

Context: Callbacks with parameters. Second step in implementing callbacks. Next step will be return values.

Description of the Change:

  1. Collect type information and tree shape during tracing
  2. Adds parameters to PythonCallOp.
  3. Bufferizes said parameters.
  4. Lowers PythonCallOp to an LLVM::CallOp where the first parameter is the function pointer, the second parameter is the number of parameters, and the rest of the parameters are pointers to memrefs.
  5. In the runtime, use va_list to collect all memrefs.
  6. Pass va_list to pybind module
  7. In pybind moulde iterate over va_list and add these to a python list
  8. In python, use the previously collected metadata to cast va_list pointers to jax arrays.
  9. Unflatten and pass the result to the user's callback.

Benefits: Callbacks with parameters.

[sc-58870]

@erick-xanadu erick-xanadu force-pushed the eochoa/2024-03-08/callbacks-parameters branch from 659b5d3 to 8cf8b3a Compare March 19, 2024 19:38
Base automatically changed from eochoa/2024-02-23/callbacks to main March 20, 2024 17:41
doc/changelog.md Outdated Show resolved Hide resolved
@rmoyard rmoyard self-requested a review March 22, 2024 15:56
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

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

It looks good to me! Mostly questions for my understanding.

Do we agree that there is no restriction on the arguments pytree structure? It could be dictionnaries for example?

doc/changelog.md Show resolved Hide resolved
doc/conf.py Show resolved Hide resolved
frontend/catalyst/pennylane_extensions.py Show resolved Hide resolved
frontend/catalyst/pennylane_extensions.py Outdated Show resolved Hide resolved
frontend/catalyst/pennylane_extensions.py Outdated Show resolved Hide resolved
mlir/include/Catalyst/IR/CatalystOps.td Outdated Show resolved Hide resolved
runtime/lib/capi/RuntimeCAPI.cpp Show resolved Hide resolved
@rmoyard rmoyard added the requires-wheel-builds Pull Requests will need wheel building job successful before being merged label Mar 22, 2024
@erick-xanadu
Copy link
Contributor Author

@rmoyard I don't think build wheels are required here. I am not changing the build system. The wheels were checked in the previous merge.

@rmoyard rmoyard added ci:build-wheels Run the wheel building workflows on this Pull Request and removed requires-wheel-builds Pull Requests will need wheel building job successful before being merged ci:build-wheels Run the wheel building workflows on this Pull Request labels Mar 22, 2024
@erick-xanadu
Copy link
Contributor Author

Do we agree that there is no restriction on the arguments pytree structure? It could be dictionnaries for example?

As long as they implement the pytree interface, they should work. The tests here include a dictionary.

Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

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

great job 🥇

@erick-xanadu erick-xanadu merged commit e26815c into main Mar 22, 2024
36 of 37 checks passed
@erick-xanadu erick-xanadu deleted the eochoa/2024-03-08/callbacks-parameters branch March 22, 2024 19:02
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.

None yet

2 participants