From 4dbcabab06dbe493ea69398df31bf7f265cf45f8 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Mon, 7 Nov 2022 23:34:15 +0000 Subject: [PATCH] Fix QPY handling of registers QPY previously had rather complicated logic to handle deserialisation of registers that was heavily couched in the old model of registers "owning" the bits they contained. This is no longer the data model of the circuit, so the code was naturally complex to try and make it work. Instead, in deserialisation now we just add the correct number of bit instances to the circuit at the start, and then build up any required registers by using the `indices` field. It's not clear to me how an index ever could be negative here; it's not clear how a register could be in the circuit but its bits aren't added, so it feels like there might be the possibility of a bug I've missed. --- qiskit/qpy/binary_io/circuits.py | 127 +++--------------- .../fix-qpy-register-57ed7cf2f3f67e78.yaml | 6 + .../circuit/test_circuit_load_from_qpy.py | 13 ++ 3 files changed, 35 insertions(+), 111 deletions(-) create mode 100644 releasenotes/notes/fix-qpy-register-57ed7cf2f3f67e78.yaml diff --git a/qiskit/qpy/binary_io/circuits.py b/qiskit/qpy/binary_io/circuits.py index 075ac339b182..bb78871ebc93 100644 --- a/qiskit/qpy/binary_io/circuits.py +++ b/qiskit/qpy/binary_io/circuits.py @@ -845,124 +845,29 @@ def read_circuit(file_obj, version, metadata_deserializer=None): num_registers = header["num_registers"] num_instructions = header["num_instructions"] out_registers = {"q": {}, "c": {}} + circ = QuantumCircuit( + [Qubit() for _ in [None] * num_qubits], + [Clbit() for _ in [None] * num_clbits], + name=name, + global_phase=global_phase, + metadata=metadata, + ) if num_registers > 0: - circ = QuantumCircuit(name=name, global_phase=global_phase, metadata=metadata) if version < 4: registers = _read_registers(file_obj, num_registers) else: registers = _read_registers_v4(file_obj, num_registers) - for bit_type_label, bit_type, reg_type in [ - ("q", Qubit, QuantumRegister), - ("c", Clbit, ClassicalRegister), - ]: - register_bits = set() + for bit_type_label, reg_type in [("q", QuantumRegister), ("c", ClassicalRegister)]: # Add quantum registers and bits - for register_name in registers[bit_type_label]: - standalone, indices, in_circuit = registers[bit_type_label][register_name] - indices_defined = [x for x in indices if x >= 0] - # If a register has no bits in the circuit skip it - if not indices_defined: - continue - if standalone: - start = min(indices_defined) - count = start - out_of_order = False - for index in indices: - if index < 0: - out_of_order = True - continue - if not out_of_order and index != count: - out_of_order = True - count += 1 - if index in register_bits: - # If we have a bit in the position already it's been - # added by an earlier register in the circuit - # otherwise it's invalid qpy - if not in_circuit: - continue - raise exceptions.QpyError("Duplicate register bits found") - - register_bits.add(index) - - num_reg_bits = len(indices) - # Create a standlone register of the appropriate length (from - # the number of indices in the qpy data) and add it to the circuit - reg = reg_type(num_reg_bits, register_name) - # If any bits from qreg are out of order in the circuit handle - # is case - if out_of_order or not in_circuit: - for index, pos in sorted( - enumerate(x for x in indices if x >= 0), key=lambda x: x[1] - ): - if bit_type_label == "q": - bit_len = len(circ.qubits) - else: - bit_len = len(circ.clbits) - if pos < bit_len: - # If we have a bit in the position already it's been - # added by an earlier register in the circuit - # otherwise it's invalid qpy - if not in_circuit: - continue - raise exceptions.QpyError("Duplicate register bits found") - # Fill any holes between the current register bit and the - # next one - if pos > bit_len: - bits = [bit_type() for _ in range(pos - bit_len)] - circ.add_bits(bits) - circ.add_bits([reg[index]]) - if in_circuit: - circ.add_register(reg) - else: - if bit_type_label == "q": - bit_len = len(circ.qubits) - else: - bit_len = len(circ.clbits) - # If there is a hole between the start of the register and the - # current bits and standalone bits to fill the gap. - if start > len(circ.qubits): - bits = [bit_type() for _ in range(start - bit_len)] - circ.add_bits(bit_len) - if in_circuit: - circ.add_register(reg) - out_registers[bit_type_label][register_name] = reg - else: - for index in indices: - if bit_type_label == "q": - bit_len = len(circ.qubits) - else: - bit_len = len(circ.clbits) - # Add any missing bits - bits = [bit_type() for _ in range(index + 1 - bit_len)] - circ.add_bits(bits) - if index in register_bits: - raise exceptions.QpyError("Duplicate register bits found") - register_bits.add(index) - if bit_type_label == "q": - bits = [circ.qubits[i] for i in indices] - else: - bits = [circ.clbits[i] for i in indices] - reg = reg_type(name=register_name, bits=bits) - if in_circuit: - circ.add_register(reg) - out_registers[bit_type_label][register_name] = reg - # If we don't have sufficient bits in the circuit after adding - # all the registers add more bits to fill the circuit - if len(circ.qubits) < num_qubits: - qubits = [Qubit() for _ in range(num_qubits - len(circ.qubits))] - circ.add_bits(qubits) - if len(circ.clbits) < num_clbits: - clbits = [Clbit() for _ in range(num_clbits - len(circ.clbits))] - circ.add_bits(clbits) - else: - circ = QuantumCircuit( - [Qubit() for _ in [None] * num_qubits], - [Clbit() for _ in [None] * num_clbits], - name=name, - global_phase=global_phase, - metadata=metadata, - ) + circuit_bits = {"q": circ.qubits, "c": circ.clbits}[bit_type_label] + for register_name, (_, indices, in_circuit) in registers[bit_type_label].items(): + register = reg_type( + name=register_name, bits=[circuit_bits[x] for x in indices if x >= 0] + ) + if in_circuit: + circ.add_register(register) + out_registers[bit_type_label][register_name] = register custom_operations = _read_custom_operations(file_obj, version, vectors) for _instruction in range(num_instructions): _read_instruction(file_obj, circ, out_registers, custom_operations, version, vectors) diff --git a/releasenotes/notes/fix-qpy-register-57ed7cf2f3f67e78.yaml b/releasenotes/notes/fix-qpy-register-57ed7cf2f3f67e78.yaml new file mode 100644 index 000000000000..ece1170bf3de --- /dev/null +++ b/releasenotes/notes/fix-qpy-register-57ed7cf2f3f67e78.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixed a bug in QPY (:mod:`qiskit.qpy`) where circuits containing registers + whose bits occurred in the circuit after loose bits would fail to deserialize. + See `#9094 `__. diff --git a/test/python/circuit/test_circuit_load_from_qpy.py b/test/python/circuit/test_circuit_load_from_qpy.py index 22003420da90..4117f27270ee 100644 --- a/test/python/circuit/test_circuit_load_from_qpy.py +++ b/test/python/circuit/test_circuit_load_from_qpy.py @@ -1103,3 +1103,16 @@ def test_load_with_loose_bits_and_registers(self): qpy_file.seek(0) new_circuit = load(qpy_file)[0] self.assertEqual(qc, new_circuit) + + def test_registers_after_loose_bits(self): + """Test that a circuit whose registers appear after some loose bits roundtrips. Regression + test of gh-9094.""" + qc = QuantumCircuit() + qc.add_bits([Qubit(), Clbit()]) + qc.add_register(QuantumRegister(2, name="q1")) + qc.add_register(ClassicalRegister(2, name="c1")) + with io.BytesIO() as fptr: + dump(qc, fptr) + fptr.seek(0) + new_circuit = load(fptr)[0] + self.assertEqual(qc, new_circuit)