-
Notifications
You must be signed in to change notification settings - Fork 603
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
Add qml.capture.to_catalsyt
conversion function
#5771
Conversation
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
…neAI/pennylane into plxpr-capture-operations
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
…neAI/pennylane into plxpr-capture-operations
Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Hello. You may have forgotten to update the changelog!
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5771 +/- ##
==========================================
- Coverage 99.67% 99.29% -0.39%
==========================================
Files 414 415 +1
Lines 39463 39316 -147
==========================================
- Hits 39336 39039 -297
- Misses 127 277 +150 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @albi3ro ! 🎉
Had some minor initial comments, will go through tests in a second round.
|
||
|
||
def to_catalyst(jaxpr: jax.core.Jaxpr) -> jax.core.Jaxpr: | ||
"""Convert pennylane variant jaxpr to catalyst variant jaxpr. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're using "plxpr" in so many places, we could go for that name and change it once a public name has been decided.
call_jaxpr={ lambda ; c:f64[]. let | ||
qdevice[ | ||
rtd_kwargs={'shots': 0, 'mcmc': False, 'num_burnin': 0, 'kernel_name': None} | ||
rtd_lib=/Users/christina/Prog/catalyst/frontend/catalyst/utils/../../../runtime/build/lib/librtd_lightning.dylib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we care to redact this kind of information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh... good point.
) | ||
n_wires = eqn.params["n_wires"] | ||
|
||
orig_wires = eqn.invars[-n_wires:] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we support zero-wires operators already? 🤔
Should be the first method called when populating the catalyst xpr. | ||
""" | ||
self._num_device_wires = len(device.wires) | ||
self._shots = device.shots |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this capture the scenario where a QNode received the shots kwarg? I suppose so, because the device temporarily has changed shots?
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
…into to-catalyst
Closed in favor of PennyLaneAI/catalyst#837 |
Context:
Previous PR's have integrated operators, measurements, and qnodes with jax primitives, allowing a full workflow to be captured with the
jax.make_jaxpr
function.To start to integrate
Description of the Change:
Benefits:
Improved integration of pennylane and catalyst.
Possible Drawbacks:
This route is the naive route of first converting to plxpr and then converting catalxpr. Another route would involve natively capturing the catalxpr itself from the original user code. But that would involve deeper coupling with jax internals and extending it with custom
CatalystTracer
andCatalystTrace
objects.This current version also only supports a limited gateset that is already natively supported by catalyst, and will not be able to handle generic templates/ more complicated operations.
We are also introducing a complicated dependency tree in this PR. So far we have the core pl objects (operations, measurements, qnode) depending on the capture module. Then catalyst depends on our pennylane infrastructure.
With this new PR, we are introducing a catalyst dependency to the
capture
module. This makes imports difficult if we want to avoid circular dependencies. I would like to propose either:to_catalyst
to the catalyst packageto_catalyst
to theqml.compiler
moduleRelated GitHub Issues:
[sc-61537]