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

ZPowGate to QASM false conversion #6726

Closed
ACE07-Sev opened this issue Sep 13, 2024 · 8 comments
Closed

ZPowGate to QASM false conversion #6726

ACE07-Sev opened this issue Sep 13, 2024 · 8 comments
Assignees
Labels
area/interop area/qasm kind/bug-report Something doesn't seem to work. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@ACE07-Sev
Copy link

Description of the issue

Greetings there,

Hope all are well. I am trying to implement the quantum_shannon_decomposition.py in qiskit, and to do that I need to represent ZPowGate using gates such as Ry, Rz, global phase, and such. To see what the conversion is I used the cirq.qasm() function, however, the conversion is wrong as can be seen below.

How to reproduce the issue

import cirq
import numpy as np
from numpy.testing import assert_almost_equal
from qiskit import QuantumCircuit
from qiskit.quantum_info import Operator

n_qubits = 1
cirq_qc = cirq.Circuit()
qubits = [cirq.NamedQubit(f'q{i}') for i in range(n_qubits)]

# No global shift case
cirq_qc.append(cirq.ZPowGate(exponent=1.5/np.pi).on(qubits[0]))

qasm_str = cirq.qasm(cirq_qc)

qiskit_qc = QuantumCircuit.from_qasm_str(qasm_str)

qiskit_U = Operator(qiskit_qc).data
cirq_U = cirq.unitary(cirq_qc)

assert_almost_equal(qiskit_U, cirq_U, 8)
During handling of the above exception, another exception occurred:

AssertionError                            Traceback (most recent call last)
Cell In[78], [line 21](vscode-notebook-cell:?execution_count=78&line=21)
     [18](vscode-notebook-cell:?execution_count=78&line=18) qiskit_U = Operator(qiskit_qc).data
     [19](vscode-notebook-cell:?execution_count=78&line=19) cirq_U = cirq.unitary(cirq_qc)
---> [21](vscode-notebook-cell:?execution_count=78&line=21) assert_almost_equal(qiskit_U, cirq_U, 8)

File /usr/lib/python3.11/contextlib.py:81, in ContextDecorator.__call__.<locals>.inner(*args, **kwds)
     [78](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/usr/lib/python3.11/contextlib.py:78) @wraps(func)
     [79](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/usr/lib/python3.11/contextlib.py:79) def inner(*args, **kwds):
     [80](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/usr/lib/python3.11/contextlib.py:80)     with self._recreate_cm():
---> [81](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/usr/lib/python3.11/contextlib.py:81)         return func(*args, **kwds)

File ~/Documents/GitHub/QICKIT/.venv/lib/python3.11/site-packages/numpy/testing/_private/utils.py:517, in assert_almost_equal(actual, desired, decimal, err_msg, verbose)
    [515](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/home/ace07/Documents/GitHub/QICKIT/.temp/~/Documents/GitHub/QICKIT/.venv/lib/python3.11/site-packages/numpy/testing/_private/utils.py:515)         assert_almost_equal(actuali, desiredi, decimal=decimal)
    [516](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/home/ace07/Documents/GitHub/QICKIT/.temp/~/Documents/GitHub/QICKIT/.venv/lib/python3.11/site-packages/numpy/testing/_private/utils.py:516)     except AssertionError:
--> [517](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/home/ace07/Documents/GitHub/QICKIT/.temp/~/Documents/GitHub/QICKIT/.venv/lib/python3.11/site-packages/numpy/testing/_private/utils.py:517)         raise AssertionError(_build_err_msg())
    [519](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/home/ace07/Documents/GitHub/QICKIT/.temp/~/Documents/GitHub/QICKIT/.venv/lib/python3.11/site-packages/numpy/testing/_private/utils.py:519) if isinstance(actual, (ndarray, tuple, list)) \
    [520](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/home/ace07/Documents/GitHub/QICKIT/.temp/~/Documents/GitHub/QICKIT/.venv/lib/python3.11/site-packages/numpy/testing/_private/utils.py:520)         or isinstance(desired, (ndarray, tuple, list)):
    [521](https://vscode-remote+wsl-002bubuntu.vscode-resource.vscode-cdn.net/home/ace07/Documents/GitHub/QICKIT/.temp/~/Documents/GitHub/QICKIT/.venv/lib/python3.11/site-packages/numpy/testing/_private/utils.py:521)     return assert_array_almost_equal(actual, desired, decimal, err_msg)

AssertionError: 
Arrays are not almost equal to 8 decimals
 ACTUAL: array([[0.73168887-0.68163876j, 0.        +0.j        ],
       [0.        +0.j        , 0.73168887+0.68163876j]])
 DESIRED: array([[1.       +0.j        , 0.       +0.j        ],
       [0.       +0.j        , 0.0707372+0.99749499j]])

Cirq version

cirq-core == 1.4.1
@ACE07-Sev ACE07-Sev added the kind/bug-report Something doesn't seem to work. label Sep 13, 2024
@ACE07-Sev
Copy link
Author

For the time being, I'd appreciate guidance on how I can implement ZPowGate with both exponent and global_shift using the rotation gates mentioned and global phase gate.

@NoureldinYosri NoureldinYosri added the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Sep 18, 2024
@verult verult added triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Oct 2, 2024
@ACE07-Sev
Copy link
Author

ACE07-Sev commented Oct 9, 2024

So, I got some decompositions for XPow, YPow, and ZPow, but having some trouble with their controlled versions. I am for instance trying the decomposition for a controlled XPow gate, and not quite sure what the decomposition would be.

For XPow itself I did

self.RX(exponent * np.pi, qubit_indices)
self.GlobalPhase(exponent * np.pi/2)

if global_shift != 0:
    self.GlobalPhase(global_shift * np.pi/2)

The tricky part is that the controlled version wouldn't be

self.CRX(exponent * np.pi, control_index, target_index)
self.GlobalPhase(exponent * np.pi/2)

if global_shift != 0:
    self.GlobalPhase(global_shift * np.pi/2)

It would need a controlled global phase gate too, which I don't think exists, as global phase is not really a gate, but more of a global correction. I don't really know how a conditional global phase gate could be implemented. IIRC qiskit just calculates the unitary when it creates the conditional version of it, so I'd really try to stay away from gates that use conditional global phase.

@ACE07-Sev
Copy link
Author

The only solution I can immediately think of is implementing its underlying unitary matrix instead, and decomposing it to a qubit qubit sequence of gates just like any other two qubit gates. This is not ideal, as the explicit decomposition could be more efficient.

@ACE07-Sev
Copy link
Author

I'll try to clean up and make a PR on the common_gates.py file for ZPow, XPow, and YPow. However, I don't like that they need a global phase gate to be representable with i.e., qiskit. As mentioned above, it makes the controlled versions rather tricky to implement without just offloading it to the unitary decomposition module.

@ACE07-Sev
Copy link
Author

ACE07-Sev commented Oct 9, 2024

I'm playing around with the idea of applying the unitary of the global phase to an actual qubit. So, it sounds very stupid as global phase is a property that's applied to the entire system, but from some testing, if we apply the global phase as a single qubit gate (its unitary representation) to any of the qubits, it behaves the same way as applying a global phase gate.

This way, we can account for global phase I think a tad easier. I did question myself ALOT about this idea, and still am, but I made sure the code was correct and the equality was properly checked. I also did the check for gradual accumulation of the global phases and it basically behaves the same way, you'd apply another and another gate.

from qiskit import QuantumCircuit
from qiskit.quantum_info import Operator

qc_1 = QuantumCircuit(3)
qc_2 = QuantumCircuit(3)
qc_3 = QuantumCircuit(3)
qc_4 = QuantumCircuit(3)

# Define GHZ state
qc.h(0)
qc.cx(0, 1)
qc.cx(0, 2)

# Apply global phase
global_phase_angle = 1/3
global_phase = np.exp(1j * global_phase_angle) * np.identity(2)

qc_1.global_phase = global_phase_angle
qc_2.unitary(global_phase, [0])
qc_3.unitary(global_phase, [1])
qc_4.unitary(global_phase, [2])

# Define the unitary operators for each circuit
u1 = Operator(qc_1).data
u2 = Operator(qc_2).data
u3 = Operator(qc_3).data
u4 = Operator(qc_4).data

# Ensure all are equal
assert np.all([np.all(u1 == u2), np.all(u1 == u3), np.all(u1 == u4)])

P.S : If I have done some really stupid mistake, I apologize in advance.

P.S : I think I can account for the controlled global phase by fixing the relative phase of the controlled indices using a MCPhase.

@dstrain115 dstrain115 added triage/discuss Needs decision / discussion, bring these up during Cirq Cynque and removed triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on labels Dec 31, 2024
@dstrain115
Copy link
Collaborator

dstrain115 commented Dec 31, 2024

Raising this on cirq discuss again. It looks like all of these are due to global phase.

For instance, changing the last line from
assert_almost_equal(qiskit_U, cirq_U, 8)
to
cirq.testing.assert_allclose_up_to_global_phase(qiskit_U, cirq_U, atol=1e-6) will work.

It is true that the unitaries will be different if you use a controlled version of that unitary, but it looks like the decomposition of the controlled XPowGate into qasm actually creates a decomposition, it doesn't do a 'ctrl' operation on 'rz' (which could be off). For example, adding ".controlled_by" in the above code to generate a CZPowGate creates this qasm:

// Gate: CZ**0.477464829275686
u3(pi*0.5,0,pi*0.9513043835) q[0];
u3(pi*0.5,0,pi*0.4513043835) q[1];
sx q[0];
cx q[0],q[1];
rx(pi*0.2612675854) q[0];
ry(pi*0.5) q[1];
cx q[1],q[0];
sxdg q[1];
s q[1];
cx q[0],q[1];
u3(pi*0.5,pi*0.2874280311,pi*1.0) q[0];
u3(pi*0.5,pi*0.7874280311,pi*1.0) q[1];

So, the qiskit and cirq gates are only off by a global phase. I don't think we want to try to support keeping the global phases in sync, as that will be difficult, clutter up the resulting qasm with a bunch of superfluous 'gphase' commands, and be functionally equivalent.

There are some other related issues though: See #5959 #6488

@pavoljuhas
Copy link
Collaborator

sync meeting - TODO - verify that a to/from qasm roundtrip produces the same unitary for cirq.
If so we can close the issue as qiskit may have a different convention for global phase.

@NoureldinYosri NoureldinYosri added triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Jan 22, 2025
@mhucka
Copy link
Contributor

mhucka commented Jan 22, 2025

Discussed in Cirq Cynq 2025-01-22.

For this specific case, the consensus is that the behavior is the preferable behavior out of the possible options; however, there are other related situations (the issues #5959 and #6488) that need to be addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/interop area/qasm kind/bug-report Something doesn't seem to work. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
Status: Done
Development

No branches or pull requests

6 participants