Skip to content
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

QuantumCircuit.compose does not raise an error with invalid qubits argument #4970

Closed
georgios-ts opened this issue Aug 23, 2020 · 2 comments · Fixed by #11451
Closed

QuantumCircuit.compose does not raise an error with invalid qubits argument #4970

georgios-ts opened this issue Aug 23, 2020 · 2 comments · Fixed by #11451
Labels
bug Something isn't working

Comments

@georgios-ts
Copy link
Contributor

Information

  • Qiskit Terra version: master @ b1c9a23
  • Python version: 3.7.1
  • Operating system: osx

What is the current behavior?

QuantumCircuit.compose runs normally if qubits argument has duplicate values. If you try to compose two circuits after converting to dag, DAGCircuit.compose raises an error. Introduced after #4762 .

Steps to reproduce the problem

n = 3
qc = QuantumCircuit(n)
qc.h(range(n))

m = 2
circ = QuantumCircuit(m)
circ.z(range(m))
circ.cx(0, 1)

new_circuit = qc.compose(circ, qubits=[1, 1])
print(new_circuit)


     ┌───┐          
q_0: ┤ H ├──────────
     ├───┤┌───┐┌───┐
q_1: ┤ H ├┤ Z ├┤ Z ├
     ├───┤└───┘└───┘
q_2: ┤ H ├──────────
     └───┘   

But

dag_qc   = circuit_to_dag(qc)
dag_circ = circuit_to_dag(circ)

dag = dag_qc.compose(dag_circ, 
                     qubits=[1, 1], inplace=False)


raises DAGCircuitError

What is the expected behavior?

As duplicate values in qubits is not a valid input, QuantumCircuit.compose should raise an error.

Suggested solutions

Similar to DAGCircuit.compose

        # Check the edge_map for duplicate values
        if len(set(edge_map.values())) != len(edge_map):
              raise CircuitError("duplicates in wire_map")
@georgios-ts georgios-ts added the bug Something isn't working label Aug 23, 2020
@Cryoris
Copy link
Contributor

Cryoris commented Sep 19, 2020

I'm a little hesitant to adding checks in the compose method because it might slow down the usage and this use-case is obviously a wrong input, so that users won't be surprised if it doesn't work. But if the DAGCircuitError is too cryptic, we could improve it there? This would not give us additional overhead when users use compose correctly but would still give a proper error message.

Edit: Maybe it would be better to actually do include the check in compose. If we notice it causes performance issues we can also add a flag that deactivates the checks or use a private method like _append?

@mrvee-qC
Copy link

Update for Santa:elf:- QGoGP

✅ Error reproducible! Has the same 'duplicates in wire map' DAGCircuitError raised as mentioned by OP

from qiskit.converters import circuit_to_dag
from qiskit import QuantumCircuit

n = 3
qc = QuantumCircuit(n)
qc.h(range(n))

m = 2
circ = QuantumCircuit(m)
circ.z(range(m))
circ.cx(0, 1)

new_circuit = qc.compose(circ, qubits=[1, 1])
print(new_circuit)
dag_qc   = circuit_to_dag(qc)
dag_circ = circuit_to_dag(circ)

dag = dag_qc.compose(dag_circ, 
                     qubits=[1, 1], inplace=False)

Screenshot

Screenshot_90

Will update comment/add more comments if cause is found/fixed!

Python version 3.9.7
qiskit-terra version: 0.19.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants