-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Make CircuitData
inner data accessible in Rust
#12766
Conversation
- Make `PackedInstruction` public.
One or more of the following people are relevant to this code:
|
CircuitData
inner data accessibleCircuitData
inner data accessible in Rust
Pull Request Test Coverage Report for Build 9910126058Details
💛 - Coveralls |
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.
This seems generally very sensible to me. Will CircuitData::iter
be enough - will you not need to access the actual qubits / clbits at some point? If so, we'll not get far without having access to the interners as well.
For the time being I only need access to the instructions name and the number of qubits affected by it. Although it will be needed to know what qargs and/or cargs is the instruction affecting, I'd rather wait for #12730 to merge and do a follow up to this to use the newer architecture your rebalancing will introduce. |
I've also come across this but would need access to the qubits and clbits. Couldn't we just already include that info in this PR and extend iter to something like pub fn iter(&self) -> impl Iterator<Item = (&PackedInstruction, &Vec<Qubit>, &Vec<Clbit>)> {
self.data.iter().map(|inst| {
(
inst,
self.qargs_interner.intern(inst.qubits_id).value,
self.cargs_interner.intern(inst.clbits_id).value,
)
})
} We could argue that iterating over circuit data should not just give the instructions but all the information (including qubits/clbits) 🙂 |
Julien: what you're getting at here is kind of the "middle type" between Can you give a bit more context at what exactly you're doing that wants it? Some points: in general, we want to avoid doing work that we don't always need to do. Not all uses of the iterator actually need to unwrap the qubit/clbit slices, which means we don't need to look them up at all, and don't need to copy around the references. Having access to the intern index gives the most performant access to the qubits/clbits in situations like copy, etc - if you have the intern key and access to the interner, you can just ask for it when you need it (my comment above). That's not at all to say that there aren't uses for a middle, fully unwrapped type as a convenience (why I'm asking), but we most likely don't want the non-critical-loop convenience method to be the only thing available. |
Just as a very rough example, consider lots of block-collection passes for peephole quantum optimisation. They don't care what the clbits are other than if they're 0 or not; for those, we don't need to waste time and memory unpacking the clbit from the intern index on each iteration through. |
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 Ray.
From offline discussion: Julien's going to take a look at reimplementing QuantumCircuit.compose
with an inner CircuitData::compose
, which will immediately alleviate some of the things he's talking about, but is also an entry point to us figuring out more of what we might need with the "middle type".
The concerns are still real, but I don't think we need to address them immediately as part of this PR, since we don't know exactly what path we'll take.
* Add: `iter()` method to `CircuitData` - Make `PackedInstruction` public. * Docs: Remove "rust-native method" from docstring.
Summary
The following commits aim to add a way to iterate through all the instructions contained in
CircuitData
to make these instructions accessible through Rust.Details and comments
iter()
method that iterates through the instructions inCircuitData
in Rust.PackedInstruction
public:This class is being made public and reorganized by Rebalance
CircuitInstruction
andPackedInstruction
#12730.Use cases:
In #12585 there's a temporary struct called
CircuitRep
which aims to represent the circuit. This struct has adata()
method that retrieves the data from the circuit once and stores it as aVec<CircuitInstruction>
.https://github.com/raynelfss/qiskit/blob/bf9aedd72b019600b223e71f46e0107a295457b3/crates/circuit/src/equivalence.rs#L311-L319
The method is used once to create a set of instructions based on their name and the number of qubits they affect. https://github.com/raynelfss/qiskit/blob/bf9aedd72b019600b223e71f46e0107a295457b3/crates/circuit/src/equivalence.rs#L448-L452
By extracting from
CircuitData
and avoiding an extra conversion to a different DataType, it will be easier for us to access those instruction attributes from the Rust side of things without needing to invoke Python.