[DAGCircuit Oxidation] Refactor bit management in CircuitData
#12372
Conversation
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 9192883136Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| struct BitAsKey { | ||
| /// Python's `hash()` of the wrapped instance. | ||
| hash: isize, | ||
| /// The wrapped instance. | ||
| bit: PyObject, | ||
| } |
There was a problem hiding this comment.
I'm wondering if we maybe should remove one of the layers of abstraction here. It's something I've been thinking about in #12205 is that the source of truth for bits is still python but for interaction from rust space this isn't super ergonomic. I'm wondering if instead of playing games like this we try flattening the two representations. I guess the complexity is things like register and index (although #11596 will take care of that)
There was a problem hiding this comment.
I'm not sure if this is what you were getting at, but I was thinking longer term that we'll want/need to make it possible to construct circuits from Rust space without creating any Python objects at all. We might not be at that point yet, but it might to be thinking about it.
There was a problem hiding this comment.
Yeah, that's basically what I was getting at, having a pattern for making a circuit without any python interaction is what we'll want longer term. The example I have for making circuits from rust space right now still needs a py handle because we need to instantiate Qubit objects.
There was a problem hiding this comment.
I'd agree as well. BitAsKey isn't new here, though. I've only moved it from CircuitData.
Also, the source of truth for bits is CircuitData. It owns its bits, and QuantumCircuit simply "references" them.
mtreinish
added
the
Rust
|
I'm rethinking a bit of this PR (specifically the stuff with packing and interning) so I'm going to put this into draft for now. The |
|
One or more of the following people are relevant to this code:
|
Summary
This is part of the work we're doing to port
DAGCircuitto Rust. It's work I've pulled out of a local branch where I'm doing that port, in an effort to reduce the size of that eventual PR.The primary improvement that this brings is the introduction of
BitData, which encapsulates keeping Python and native bit indices in sync. It provides methods likemap_indicesandmap_bitswhich improve the ergonomics of translating between native and Python bits at PyO3 boundaries.Details and comments
QubitandClbitrepresentation.BitDatawhich encapsulates maintaining a mapping between native Rust bit types and Python bit types. This significantly cleans up the code inCircuitDataand makes it reusable forDAGCircuit.InternContextinto a more genericIndexedInternerdata structure, which implements the newInternertrait with slotted interning for anyTwhich isHashandEq.Internertrait, for defining interner semantics for containers likeIndexedInterner.The one caveat to this PR is that we now no longer share interning between
QubitandClbitindices, since this is more difficult from a typing perspective. If we want to share a singleIndexedInternerfor bothQubitandClbitoperand lists, this would be possible, but we should revisit it. For now I wanted to just focus on keeping things strongly typed so that our Rust code doesn't get overly messy at this early stage.