-
Notifications
You must be signed in to change notification settings - Fork 715
Standardize the output format of qml.specs
#8713
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
Conversation
|
Hello. You may have forgotten to update the changelog!
|
…e into chore/specs-outputs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8713 +/- ##
==========================================
- Coverage 99.37% 99.37% -0.01%
==========================================
Files 589 589
Lines 62830 62929 +99
==========================================
+ Hits 62437 62534 +97
- Misses 393 395 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…e into chore/specs-outputs
|
Just a heads up that I'll also need reviews on this follow-up PR which fixes the specs integration tests in Catalyst: PennyLaneAI/catalyst#2255 . These tests will begin to fail as soon as the changes from this PR are used with Catalyst (requires a change to the manual pin) |
albi3ro
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.
So excited to have this in! I've actually wanted something like this for a while, so quite happy to have something like this now.
mudit2812
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 great @jzaia18 .
Introduces another "resources" object into PennyLane. A followup PR should restructure these PRs such that all of them inherit from a common base class.
Is there already a story for this? If not, could you please add one?
Not sure about a story, but I think there's a whole epic for it planned for Q1, it's in the technical roadmap |
**Context:** [This PR](PennyLaneAI/pennylane#8713) in PennyLane changes how `qml.specs` returns its results. This will break some integration tests within Catalyst. **Description of the Change:** Updates tests within `frontend/test/pytest/test_specs.py` to account for the new `qml.specs` output format. **Benefits:** **Possible Drawbacks:** **Related GitHub Issues:** PennyLaneAI/pennylane#8713 [sc-104965] --------- Co-authored-by: Mudit Pandey <[email protected]>
Context:
qml.specsmakes very few guarantees on the shape of its output. This PR aims to make a single unified out type forqml.specs, which contains all of (and only) the information needed for circuit inspection.Description of the Change:
SpecsResourcesandCircuitSpecsclassesspecsfunctions throughout PennyLane to make use of the new classes_count_resourcesto use the Catalyst-style resource names for multi-controlled gates. e.g. a 10-controlledIsingXXis now shown as10C(IsingXX)and no longer justC(IsingXX). This matches how resources are counted in Catalyst.Benefits:
Cleaner and more consistent
specsreturns.Happens to also fix #8048
Possible Drawbacks:
Introduces another "resources" object into PennyLane. A followup PR should restructure these PRs such that all of them inherit from a common base class.
Removes some information which used to be included in
qml.specs, such asnum_observables(now migrated into the resources'measurements) or various gradient information.Related GitHub Issues:
[sc-104965]
#8048