Converters, quantum_info, assembler, scheduler and circuit library updates for optional registers. #5504
Conversation
5c5ac70 to
6a9408e
Compare
6a9408e to
4df285c
Compare
4df285c to
31cf7a5
Compare
There was a problem hiding this comment.
This LGTM, but the only thing that sticks out to me is a potential optimization, this PR is basically building a (qu)bit map dictionary everywhere things are accessed by bit. I'm wondering it makes more sense to stick that in the circuit itself so all these places can just access it instead of creating it on the fly everywhere.
I know that means we have 3 views of a the bits in a circuit which seems like a headache. But maybe we can combine _qubit_set and the map (like add a getter for qubit set that returns set(map)).
| @@ -92,14 +81,13 @@ def find_bit_position(bit): | |||
| if gate.num_qubits > 0: | |||
| q = QuantumRegister(gate.num_qubits, 'q') | |||
|
|
|||
| qubit_map = {bit: q[idx] for idx, bit in enumerate(circuit.qubits)} | |||
There was a problem hiding this comment.
I'm wondering if it makes sense to carry this around in the circuit object itself (maybe as a private). Pretty much anything that is going from a circuit to something else is now building this dict.
I agree, and the cost of keeping these in sync was why I held off initially (this was before the addition of |
|
I would probably just do it in this PR I feel like that'll be simpler unless it has implications for the next patches in the series. |
I'm a little worried about introducing more conflicts with the remaining PRs, maybe as part of #5868 ? |
Ok sure sounds good, then this LGTM |
Summary
Following #5498 , updates converters, quantum_info, assembler, circuit library and scheduler to support optional registers, and to prefer inspecting bits over registers where possible.
Details and comments
on-hold until #5498 merges.Commits in this PR (without #5498 and #5486 ): https://github.com/Qiskit/qiskit-terra/pull/5504/files/18c95db0f2daef7fe52674ca1ec7fb5e7f1eae3e..5c5ac703ff05a25bdea6ae57346440d15d45033f