qiskit icon indicating copy to clipboard operation
qiskit copied to clipboard
verified publisher icon Qiskit

Update MCX synthesis to use `PhaseGate` instead of `U1Gate`

Open Cryoris opened this issue 1 year ago • 1 comments

Summary

We should update the synthesis methods to use the PhaseGate instead of the IBM-legacy U1Gate, if possible. See https://github.com/Qiskit/qiskit/pull/12961#discussion_r1719719621.

Cryoris avatar Aug 19 '24 06:08 Cryoris

RZGate also should be fine if you prefer to centralise on the rx/ry/rz set to make the logic simpler - just needs a couple of local operations to account for the phase difference.

jakelishman avatar Aug 21 '24 11:08 jakelishman

Hello.. if nobody is working on this, I would like to take this up.

Ak-ash22 avatar Jan 12 '25 10:01 Ak-ash22

Hi @Ak-ash22, yes that would be great! 🙂 You can start by having a look at the functions in https://github.com/Qiskit/qiskit/blob/5757fa0a34ea3edd4565555e2ebdecbbeb0db387/qiskit/synthesis/multi_controlled/mcx_synthesis.py and see whether any tests fail if substituting the U1/CU1/MCU1Gate for the Phase/CPhase/MCPhaseGate 🙂

Cryoris avatar Jan 13 '25 07:01 Cryoris

Hi @Cryoris @jakelishman I changed the CU1/MCU1Gate to the CPhase/MCPhaseGate . I didnt find the file mcx_synthesis.py using U1Gate in it though. For these changes in the file, the test seems to be holding up. I did the following tests:

  1. tox -eblack
  2. tox -elint-incr
  3. tox -epy ----- this throws a error in the file qiskit/qiskit/__init__.py with some module qiskit._accelerate.circuit, which I think is not because of my changes.

Is this all good? Am I missing something?

Ak-ash22 avatar Jan 16 '25 07:01 Ak-ash22

Could you open a draft PR with your changes? Without code to look at it's difficult to give feedback. 🙂

Cryoris avatar Jan 16 '25 09:01 Cryoris