Support PBC Depth within SpecsResources#9703
Conversation
|
Hello. You may have forgotten to update the changelog!
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9703 +/- ##
==========================================
- Coverage 99.47% 99.45% -0.02%
==========================================
Files 629 629
Lines 69155 69197 +42
==========================================
+ Hits 68789 68818 +29
- Misses 366 379 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| depth_str = ( | ||
| _count_to_str(self.depth, markdown_safe=True) | ||
| if self.depth is not None | ||
| else "Not computed" |
There was a problem hiding this comment.
For non-pbc circuit, do we need to show PBC depth as well? it seems we return "Not computed".
There was a problem hiding this comment.
That's for ordinary circuit depth, not PBC depth
There was a problem hiding this comment.
I can add a comment to make this clearer in the code
| if depths := resources.get("pbc_depth"): | ||
| if "depth_0" in depths and "depth_1" in depths: | ||
| pbc_depth = (depths["depth_0"], depths["depth_1"]) |
There was a problem hiding this comment.
I briefly brought this up on slack, but would it make sense to frame these more generally?
I wonder if we could make the labels a bit more descriptive, for instance
Depth-any-comm,Depth-qw-comm, or similar.The (PBC) header could also be removed if we wanted a single section for all our depth metrics, and perhaps others in the future. The qubit-wise commutativity depth for instance is not exclusive to the PBC model, one could compute it on regular gate circuits as well.
Maybe the depth labels themselves can be grabbed from the resource data, and we don't have to assume that these are PBC specific?
Context:
PennyLaneAI/catalyst#2967 adds support for some PBC-related fields to the resource analysis output. As a temporary measure, PBC depth fields will be added to
SpecsResources. This work is related to the frontend integration ofppm_specsDescription of the Change:
Adds an optional
pbc_depthfield toSpecsResources. This field is eitherNone(in most cases, and therefore hidden) or a tuple of thedepth-0anddepth-1related to the PPMs & PPRs in the circuit's IR.Benefits:
Allows PBC depth info to be displayed to end users, which is useful to some internal researchers.
Possible Drawbacks:
These fields are not used in most cases and should not live in this class long term. This adds tech debt that we will have to repay some time next quarter when we restructure the resource class hierarchy.
Related GitHub Issues:
[sc-122858]