Rework circuit measurement handling - #1532
Conversation
…it-measurement-rework
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1532 +/- ##
==========================================
- Coverage 91.83% 91.74% -0.10%
==========================================
Files 147 147
Lines 11745 11724 -21
==========================================
- Hits 10786 10756 -30
- Misses 959 968 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
RoyStegeman
left a comment
There was a problem hiding this comment.
Thanks again @sorewachigauyo, this is nice improvement. Since I don't have a platform available to check myself, did you run the protocols to make sure the results are still sensible?
Yes, I ran all the protocols on my QPU and attached the results in original comment #1532 (comment) |
|
Thanks @sorewachigauyo I'll have a look. Did you notice the test failing? Nvm, seems it was a fluke |
|
It seems to be on an unrelated test on windows py312, the other tests were passing by then. I checked on my end before I pushed. |
RoyStegeman
left a comment
There was a problem hiding this comment.
Thanks, I think it looks good up to a few issues with inconsistent use of types.
| true_circuit.add(gates.Z(1)) | ||
| true_circuit.add(gates.Z(2)) | ||
| assert np.all(true_circuit.unitary() == transpiled_circuit.unitary()) | ||
| qubit_map = [0, 1] |
There was a problem hiding this comment.
| qubit_map = [0, 1] | |
| qubit_map: QubitMap = [0, 1] |
| if len(measure.qubits) == 1: | ||
| qid = platform_qubit_map[measure.qubits[0]] | ||
| else: | ||
| qid = tuple([platform_qubit_map[qubit_id] for qubit_id in measure.qubits]) |
There was a problem hiding this comment.
| qid = tuple([platform_qubit_map[qubit_id] for qubit_id in measure.qubits]) | |
| len(measure.qubits) == 2 | |
| qid = ( | |
| platform_qubit_map[measure.qubits[0]], | |
| platform_qubit_map[measure.qubits[1]], | |
| ) |
| result = indexed_result.result | ||
| qubit_pair: QubitPairId = indexed_result.target |
There was a problem hiding this comment.
| result = indexed_result.result | |
| qubit_pair: QubitPairId = indexed_result.target | |
| result = indexed_result.result | |
| assert isinstance(indexed_result.target, tuple) | |
| qubit_pair: QubitPairId = indexed_result.target |
| """An array where the elements are physical qubit IDs (str/int) and the indices are | ||
| logical qubit IDs | ||
| """ | ||
| ResultMap = dict[QubitId | QubitPairId, list[Counter[str]]] |
There was a problem hiding this comment.
| ResultMap = dict[QubitId | QubitPairId, list[Counter[str]]] | |
| ResultMap = dict[QubitId | QubitPairId | tuple[QubitId,...], list[Counter[str]]] |
Can't this also be tuple[QubitId,...] for the readout mitigation matrix protocol?
There was a problem hiding this comment.
| results = execute_circuits( | ||
| circuits, | ||
| [list(pair)] * len(circuits), | ||
| [pair], |
There was a problem hiding this comment.
execute_circuits now expects a list[QubitMap], so which is not what [pair] is. in principle this should be fine since you want to use the pair as a list of QubitMap, but you may want to explicitly cast it to a list list(pair).
As described in #1530, we relax the condition that assigns only one qubit per measurement register and provide handling of multiple qubits per measurement register.
From the normal flow of circuit execution, we get a map of measurement registers/gates to pulse sequences. The qubits in these registers are logical indices, so we can map them back onto the physical qubit IDs thanks to the platform mapping being available. We then reuse the original result handling but assign the state counts to the physical qubit IDs instead of the logical IDs.
TODO