-
Notifications
You must be signed in to change notification settings - Fork 714
Use Resource Estimation Module to build call graphs for QT-PL integration #8390
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
base: master
Are you sure you want to change the base?
Conversation
|
Hello. You may have forgotten to update the changelog!
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8390 +/- ##
=======================================
Coverage 99.42% 99.42%
=======================================
Files 587 587
Lines 62168 62235 +67
=======================================
+ Hits 61812 61879 +67
Misses 356 356 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
obliviateandsurrender
left a comment
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 @austingmhuang! Leaving some initial comments.
| [(#8468)](https://github.com/PennyLaneAI/pennylane/pull/8468) | ||
|
|
||
| * The custom call graphs for resource estimation have been removed. Call graphs are now | ||
| computed using the `pennylane.estimator` module, which may lead to different results. |
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.
| computed using the `pennylane.estimator` module, which may lead to different results. | |
| computed using the `pennylane.estimator` module. |
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.
Instead, you might provide a reason on why this is a good change.
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.
Suggestion: it enables scaling up to larger circuits. For example this now takes 5 seconds. It used to take 5 minutes.
import pennylane as qml
import numpy as np
shape = qml.StronglyEntanglingLayers.shape(n_layers=200, n_wires=400)
rng = np.random.default_rng(12345)
weights = rng.random(size=shape)
op = qml.StronglyEntanglingLayers(weights=weights, wires=range(400))
bloq = qml.io.ToBloq(op)
bloq.call_graph()
Jaybsoni
left a comment
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.
Changes look good to me. Are there any integration tests that show how this feature works in the general context 👍🏼.
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.
Approved from my side as soon as the changelog is updated and tests pass 👍
Jaybsoni
left a comment
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.
Looks good to me, happy to approve once we add an integration test for how this feature would work more generally. Also do we have a PR open for the wire mapping bug?
Implement the integration test and fixed the wire mapping bug here and made the appropriate changes to the test cases. |
I will update the changelog. |
Jaybsoni
left a comment
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.
Looks good to me,
Co-authored-by: Jay Soni <[email protected]>
Context:
Previously we had custom written call graphs in order to support workflows in a reasonable time frame and produce resource estimates. Now that we have the resource estimation module in Pennylane core, we can leverage that to produce the call graphs instead, rather than using the custom call graphs we had written at the inception of the integration.
Description of the Change:
Delete all the previous existing call graphs and simply get estimates from the pennylane.estimator module and convert those to Qualtran-understandable values.
Benefits:
Makes sure that any ToBloq'd pennylane operator/circuit's call graph is equivalent to the qre.estimate of the same pennylane operator/circuit.
Possible Drawbacks:
There might be some differences between the results arising from the original custom written call graphs and that from the pennylane.estimator module
Related GitHub Issues:
[sc-100485]