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

Make add_control identify known definitions by type not name #3540

Closed
dime10 opened this issue Dec 3, 2019 · 4 comments
Closed

Make add_control identify known definitions by type not name #3540

dime10 opened this issue Dec 3, 2019 · 4 comments
Labels
bug Something isn't working

Comments

@dime10
Copy link
Contributor

dime10 commented Dec 3, 2019

Information

  • Qiskit Terra version: 0.11.0.dev0+b7d6a90
  • Python version: 3.6.9
  • Operating system: Linux

What is the current behavior?

Currently when controlling a gate via gate.control(), the add_control function will check if the to be generated gate exists as special gate in extensions (such as cnot, toffoli, ect...), and create that instead of a generic one. This check is done by comparing the gate.name attribute against 'x', 'y', 'z' and so on. This can be problematic and should be replaced with a type check.

This can actually lead to a bug, for example if a gate is created from a circuit object via qc.to_gate(), and the circuit was unfortunately named (e.g. 'x'). Then the resulting gate will also have the same name, and could thus be transformed into the wrong gate upon controlling it (e.g. into a CnotGate if named 'x').

Steps to reproduce the problem

from qiskit import QuantumCircuit
qc = QuantumCircuit(3, name='x')
qc.h([0,1,2])
gate = qc.to_gate()
gate.control()

What is the expected behavior?

Suggested solutions

@1ucian0 1ucian0 added bug Something isn't working priority: high labels Dec 6, 2019
@1ucian0 1ucian0 added this to the 0.12 milestone Dec 6, 2019
@willhbang
Copy link
Contributor

I can make these changes if no one's started already

@dime10
Copy link
Contributor Author

dime10 commented Jan 12, 2020

I can make these changes if no one's started already

Sounds good, I don't know of anyone who has.

@kdk
Copy link
Member

kdk commented Jan 14, 2020

FYI, #3631 partially addresses this problem (at least for the standard gates). Implementation on this may want to wait until that merges.

@jakelishman
Copy link
Member

I'm going to close this as stale now, in part because the data model of Terra is that the gate name is treated as an opcode; a circuit named x must faithfully implement the Pauli X operation, and it would be a user error if it doesn't. As some other examples of constructs specifically built around the data model, we'd see the basis_gates argument to the transpiler, the required format of backend devices, and the modern Target.

Please feel free to re-open if there's more to discuss.

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.

6 participants