search

Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5.3k stars 2.38k forks source link

Circuit_to_instruction converter missing classical register for conditional gates #4672

Closed kdk closed 4 years ago

kdk commented 4 years ago

Information

What is the current behavior?

From https://travis-ci.com/github/Qiskit/qiskit-terra/jobs/358424742#L3190 , in some cases, the transpiler is generating conditional gates without the clbits on which they are conditioned. This was noticed in (but not necessarily caused by) code added in #4622 ( https://github.com/Qiskit/qiskit-terra/pull/4622/files#diff-dd64919667465e6a3c40b2a4ed00f8e5R117 ) when building instructions from circuits with conditional gates.

Steps to reproduce the problem

Transpile the QASM from the below stack trace at optimization_level=3 targeting FakeJohannesburg.

======================================================================
ERROR: runTest (hypothesis.stateful.QCircuitMachine.TestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/Qiskit/qiskit-terra/test/randomized/test_transpiler_equivalence.py", line 230, in equivalent_transpile
    xpiled_qc = transpile(self.qc, backend=backend, optimization_level=opt_level)
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/compiler/transpile.py", line 214, in transpile
    circuits = parallel_map(_transpile_circuit, list(zip(circuits, transpile_args)))
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/tools/parallel.py", line 106, in parallel_map
    return [task(values[0], *task_args, **task_kwargs)]
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/compiler/transpile.py", line 312, in _transpile_circuit
    output_name=transpile_config['output_name'])
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/transpiler/passmanager.py", line 214, in run
    return self._run_single_circuit(circuits, output_name, callback)
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/transpiler/passmanager.py", line 277, in _run_single_circuit
    result = running_passmanager.run(circuit, output_name=output_name, callback=callback)
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/transpiler/runningpassmanager.py", line 115, in run
    dag = self._do_pass(pass_, dag, passset.options)
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/transpiler/runningpassmanager.py", line 145, in _do_pass
    dag = self._run_this_pass(pass_, dag)
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/transpiler/runningpassmanager.py", line 157, in _run_this_pass
    new_dag = pass_.run(dag)
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/transpiler/passes/optimization/consolidate_blocks.py", line 128, in run
    unitary = UnitaryGate(Operator(subcirc))  # simulates the circuit
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/quantum_info/operators/operator.py", line 86, in __init__
    self._data = self._init_instruction(data).data
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/quantum_info/operators/operator.py", line 490, in _init_instruction
    instruction = instruction.to_instruction()
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/circuit/quantumcircuit.py", line 819, in to_instruction
    return circuit_to_instruction(self, parameter_map)
  File "/home/travis/build/Qiskit/qiskit-terra/qiskit/converters/circuit_to_instruction.py", line 119, in circuit_to_instruction
    if reg.size == c.size:
UnboundLocalError: local variable 'c' referenced before assignment

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.7.6/lib/python3.7/site-packages/hypothesis/stateful.py", line 393, in runTest
    run_state_machine_as_test(state_machine_class)
  File "/home/travis/virtualenv/python3.7.6/lib/python3.7/site-packages/hypothesis/stateful.py", line 75, in run_state_machine_as_test
    def run_state_machine_as_test(state_machine_factory, *, settings=None):
  File "/home/travis/virtualenv/python3.7.6/lib/python3.7/site-packages/hypothesis/internal/reflection.py", line 652, in accept
    return func(*bound.args, **bound.kwargs)
  File "/home/travis/virtualenv/python3.7.6/lib/python3.7/site-packages/hypothesis/stateful.py", line 200, in run_state_machine_as_test
    run_state_machine(state_machine_factory)
  File "/home/travis/virtualenv/python3.7.6/lib/python3.7/site-packages/hypothesis/stateful.py", line 92, in run_state_machine
    @given(st.data())
  File "/home/travis/virtualenv/python3.7.6/lib/python3.7/site-packages/hypothesis/core.py", line 1141, in wrapped_test
    raise the_error_hypothesis_found
  File "/home/travis/build/Qiskit/qiskit-terra/test/randomized/test_transpiler_equivalence.py", line 234, in equivalent_transpile
    raise RuntimeError(failed_qasm) from e
RuntimeError: Exception caught during transpilation of circuit: 
OPENQASM 2.0;
include "qelib1.inc";
qreg q3110[12];
qreg q3157[2];
creg c95[5];
creg c96[2];
creg c97[5];
creg c98[5];
u2(-24.508971,1.5216458) q3110[6];
cz q3110[8],q3110[7];
u2(-17.363907,-22.934491) q3110[9];
rz(-8.7083803) q3110[4];
reset q3110[7];
cy q3110[10],q3110[3];
measure q3110[4] -> c95[0];
cz q3110[3],q3110[11];
h q3110[0];
u2(-9.6786822,2.9792195) q3110[1];
cu3(-12.105011,-7.0180632,-18.792019) q3110[5],q3110[0];
u3(28.464436,-11.005153,-3.6969371) q3110[9];
cu3(23.569199,4.5776732,-2.6956357) q3110[9],q3110[0];
rz(-11.654972) q3110[8];
u3(16.36128,25.832861,28.922542) q3110[1];
cswap q3110[9],q3110[4],q3110[3];
measure q3110[4] -> c96[0];
ch q3110[6],q3110[7];
x q3110[4];
barrier q3110[7];
ccx q3110[1],q3110[9],q3110[10];
cu3(21.612624,-18.30063,1.3315713) q3110[1],q3110[7];
cy q3110[7],q3110[10];
cy q3110[11],q3110[10];
measure q3110[6] -> c97[4];
swap q3157[0],q3110[6];
rzz(22.30788) q3110[11],q3110[6];
if(c97==13) cu3(-13.732855,-10.110604,29.464678) q3110[8],q3157[0];
ccx q3110[10],q3110[9],q3110[7];
if(c97==25) u3(9.6297074,22.387267,26.921443) q3110[2];
if(c97==8) sdg q3110[3];
rx(-7.769637) q3110[3];
ccx q3110[0],q3157[1],q3110[7];

What is the expected behavior?

Suggested solutions

ewinston commented 4 years ago

The minimal code which seems to reproduce the bug is,

OPENQASM 2.0;
include "qelib1.inc";
qreg q1[2];
creg c1[1];

if(c1==1) cx q1[0], q1[1];

During transpilation the dag seems to become invalid, when running _check_condition over nodes, in consolidate_blocks. It seems consolidate_blocks relies on being able to create and simulate a circuit with a classical condition but no classical register.

A workaround would be for circuit_to_instruction to change,

if condition: -> if condition and instruction.num_clbits > 0:

although it seems strange for circuit_to_instruction to have to check both conditions.

enavarro51 commented 4 years ago

@kdk @ewinston The following diff seems to take care of the problem and passes all the auto-tests. If this seems like a reasonable approach, let me know and I'll do a PR.

--- a/qiskit/transpiler/passes/optimization/consolidate_blocks.py
+++ b/qiskit/transpiler/passes/optimization/consolidate_blocks.py
@@ -16,7 +16,7 @@

 """Replace each block of consecutive gates by a single Unitary node."""

-from qiskit.circuit import QuantumRegister, QuantumCircuit
+from qiskit.circuit import QuantumRegister, ClassicalRegister, QuantumCircuit
 from qiskit.dagcircuit import DAGCircuit
 from qiskit.quantum_info.operators import Operator
 from qiskit.quantum_info.synthesis import TwoQubitBasisDecomposer
@@ -112,12 +112,19 @@ class ConsolidateBlocks(TransformationPass):
             else:
                 # find the qubits involved in this block
                 block_qargs = set()
+                block_cargs = set()
                 for nd in block:
                     block_qargs |= set(nd.qargs)
+                    if nd.condition:
+                        block_cargs |= set(nd.condition[0])
                 # convert block to a sub-circuit, then simulate unitary and add
-                block_width = len(block_qargs)
-                q = QuantumRegister(block_width)
-                subcirc = QuantumCircuit(q)
+                q = QuantumRegister(len(block_qargs))
+                # if condition in node, add clbits to circuit
+                if len(block_cargs) > 0:
+                    c = ClassicalRegister(len(block_cargs))
+                    subcirc = QuantumCircuit(q, c)
+                else:
+                    subcirc = QuantumCircuit(q)
                 block_index_map = self._block_qargs_to_indices(block_qargs,
                                                                global_index_map)
                 basis_count = 0
kdk commented 4 years ago

Thanks @enavarro51 . I agree that looks like a reasonable fix. Can you add a test and open a PR?

enavarro51 commented 4 years ago

Will do. Is a reno required?

kdk commented 4 years ago

Will do. Is a reno required?

Since this fixes a bug from a previous release, a reno would be great.