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

enable controlled opaque gates. #3950

Closed
wants to merge 39 commits into from
Closed

Conversation

ewinston
Copy link
Contributor

@ewinston ewinston commented Mar 10, 2020

Summary

Previously attempting to create a controlled gate from an opaque gate would raise an error as described in #3565. This pr returns an opaque controlled gate for opaque gates.

fixes #3565
fixes #4010

Details and comments

This pr allows the identity gate to be controlled to handle controlled composite gates which contain the gate. The control operation applies identity gates to the controls as well.

qiskit/extensions/standard/x.py Outdated Show resolved Hide resolved
qiskit/circuit/add_control.py Outdated Show resolved Hide resolved
qiskit/circuit/add_control.py Outdated Show resolved Hide resolved
@Cryoris
Copy link
Contributor

Cryoris commented Mar 31, 2020

I think this doesn't correctly fix #3565, or at least the example I tried:

from qiskit import transpile, QuantumCircuit
from qiskit.circuit import Gate

basis_gates = ['u1', 'u3', 'cx']
gate = Gate('my_gate', 1, [])
gate._definition = []

circuit = QuantumCircuit(2)
circuit.append(gate.control(), [0, 1], [])
transpile(circuit, basis_gates=basis_gates)

still resolves in

QiskitError: "Cannot unroll the circuit to the given basis, ['u1', 'u3', 'cx']. No rule to expand instruction cmy_gate.

@ewinston Are you actively working on this? In a few test cases for arithmetic circuits for the circuit library I had the case that I'm controlling empty gates. I could work around there by checking first if the definition is empty, but if this is on the way to be merged I can avoid that 🙂

@ewinston
Copy link
Contributor Author

ewinston commented Apr 7, 2020

@Cryoris This seems to be due to the transpiler expecting opaque gates to have an empty list for definition instead of None. Controlled opaques where changed to match.

@ajavadia
Copy link
Member

ajavadia commented Apr 22, 2020

@ewinston can you update this branch?
also i noticed UnitaryGates don't return an instance of ControlledGate. they should I think

Copy link
Member

@ajavadia ajavadia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm good with this but will let @Cryoris or @1ucian0 do a final check. Thanks.

test/python/circuit/test_controlled_gate.py Outdated Show resolved Hide resolved
test/python/circuit/test_controlled_gate.py Outdated Show resolved Hide resolved
@CLAassistant
Copy link

CLAassistant commented Sep 4, 2020

CLA assistant check
All committers have signed the CLA.

@mtreinish
Copy link
Member

Is this still active? I'm thinking this has been superseded by the AnnotedOperation work: #9846 which enables controlling arbitrary operations and leaves the synthesis to the transpiler.

@jakelishman
Copy link
Member

Given the above comment, and the AnnotatedGate work which should supersede this, I'll mark this as closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants