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

Complete the integration of multi-controlled qubit gates #3639

Closed
6 of 7 tasks
Cryoris opened this issue Dec 19, 2019 · 5 comments
Closed
6 of 7 tasks

Complete the integration of multi-controlled qubit gates #3639

Cryoris opened this issue Dec 19, 2019 · 5 comments
Assignees
Labels
type: enhancement It's working, but needs polishing

Comments

@Cryoris
Copy link
Contributor

Cryoris commented Dec 19, 2019

After migrating to Terra from Aqua, the multi-controlled qubit gates are missing tests. Since tests for these gates still exist in Aqua, they could simply be migrated as well.
Is there a reason why they have not yet been imported to Terra?

Also, they are not yet gate objects. Once they are, we can also use these to override the control of the Toffoli and rotation gates to return these, more efficient controlled gate versions if the number of control qubits is larger than 1 (related to #3631).

This concerns the following gates:

mct mcrx mcry mcrz rccx rcccx

Plan:

  • Move the tests for these gates to Terra
  • Implement the relative-phase Toffoli gates
  • Implement the multi-controlled U1 gate
  • Implement the multi-controlled X gate
  • Implement the multi-controlled rotations
  • Port the multi-controlled multi-target X gate from Aqua
  • Port the boolean logic gates from Aqua
@ewinston
Copy link
Contributor

The ControlledGate implementation uses these gates for the rotation and Toffoli gates so it should be as effecient. Perhaps if these gates were converted to proper gates, however, it would simplify the code in add_control further. Additionally there are tests for multicontrolled gates although perhaps not everything that is tested in aqua so that could be good to add.

Something else which could be added is their support for ancilla.

@1ucian0 1ucian0 added the type: enhancement It's working, but needs polishing label Dec 21, 2019
@kdk
Copy link
Member

kdk commented Jan 13, 2020

When the tests are migrated, we should include https://github.com/Qiskit/qiskit-aqua/blob/master/qiskit/aqua/circuits/gates/boolean_logical_gates.py and their tests so they can be removed from aqua.

@frankharkins
Copy link
Member

frankharkins commented Apr 1, 2020

@Cryoris will the multi-qubit toffolis have the behaviour:

qc.mcrz(math.pi, [0,1,2], 3)

(i.e. take integers as well as / instead of Qubit objects)? It would be good to have this to be consistent with the other QuantumCircuit methods such as qc.x(0).

@Cryoris
Copy link
Contributor Author

Cryoris commented Apr 1, 2020

Yes, they will 🙂

@Cryoris
Copy link
Contributor Author

Cryoris commented Apr 1, 2021

Closing in favor of #4451.

@Cryoris Cryoris closed this as completed Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement It's working, but needs polishing
Projects
None yet
Development

No branches or pull requests

5 participants