Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5.13k stars 2.35k forks source link

cu3 simulated matrix false on unitary simulator #546

Closed nelimee closed 5 years ago

nelimee commented 6 years ago

Informations

What is the current behavior?

The cu3 gate simulated with the local_unitary_simulator does not produce the expected matrix for a cu3 gate.

Steps to reproduce the problem

Execute the script:

import qiskit
from qiskit import available_backends, get_backend,\
    execute, QuantumRegister, ClassicalRegister, QuantumCircuit
import numpy as np

def round_to_zero(vec, tol=2e-15):
    vec.real[abs(vec.real) < tol] = 0.0
    vec.imag[abs(vec.imag) < tol] = 0.0
    return vec

def controlled(U):
    n = U.shape[0]
    return np.vstack((np.hstack((np.identity(n), np.zeros((n,n)))),
                      np.hstack((np.zeros((n,n)), U))))

def U(theta, phi, lam):
    return np.array([
        [np.cos(theta/2),
         -np.exp(1.j*lam)*np.sin(theta/2)],
        [np.exp(1.j*phi)*np.sin(theta/2),
         np.exp(1.j*(phi+lam))*np.cos(theta/2)]
    ])

Q_SPECS = {
    "name": "TestCU3",
    "circuits": [
        {
            "name": "TestCU3",
            "quantum_registers": [
                {
                    "name": "ctrl",
                    "size": 1
                },
                {
                    "name": "qb",
                    "size": 1
                },
            ],
            "classical_registers": []
        }
    ],
}
Q_program = qiskit.QuantumProgram(specs=Q_SPECS)

circuit = Q_program.get_circuit("TestCU3")
qb = Q_program.get_quantum_register("qb")
ctrl = Q_program.get_quantum_register("ctrl")

theta, phi, lam = np.pi, np.pi/2, 3*np.pi/2

circuit.cu3(theta, phi, lam, ctrl[0], qb[0])

unitary_sim = get_backend('local_unitary_simulator')
res = execute([circuit], unitary_sim).result()
unitary = round_to_zero(res.get_unitary())
wanted_unitary = round_to_zero(controlled(U(theta, phi, lam)))

print("Unitary matrix from simulator:\n", unitary, sep='')
print("Unitary matrix from theory:\n", wanted_unitary, sep='')
print("Unitary matrices are the same:", np.allclose(unitary, wanted_unitary))

Output on my machine:

Unitary matrix from simulator:
[[1.+0.j 0.+0.j 0.+0.j 0.+0.j]
 [0.+0.j 0.+0.j 0.+0.j 0.-1.j]
 [0.+0.j 0.+0.j 1.+0.j 0.+0.j]
 [0.+0.j 0.-1.j 0.+0.j 0.+0.j]]
Unitary matrix from theory:
[[1.+0.j 0.+0.j 0.+0.j 0.+0.j]
 [0.+0.j 1.+0.j 0.+0.j 0.+0.j]
 [0.+0.j 0.+0.j 0.+0.j 0.+1.j]
 [0.+0.j 0.+0.j 0.+1.j 0.+0.j]]
Unitary matrices are the same: False

What is the expected behavior?

The two printed matrices should be equal.

Suggested solutions

The problem might be in:

  1. The unitary simulator.
  2. The definition of the cu3 gate in qiskit.extensions.standard.header.

I don't have any solution for the moment.

chriseclectic commented 6 years ago

It appears there that the definition of cu3 in the standard header is actually implementing the a controlled-U where U = exp( -1j * (phi + lam)/2) * U3(theta, phi, lam). So there is a local phase which wouldn't matter for a single u3 (in this case it would be a global phase), but is important when implementing the controlled gate.

(Less important but you swapped the target and control qubits between the two cases, the "standard" cnot in qiskit is CX(0, 1) = [[1, 0, 0, 0], [0, 0, 0, 1], [0, 0, 1, 0], [0, 1, 0, 0]] since qubit 0 is to the right of the tensor product).

jaygambetta commented 6 years ago

This is in the tutorial and i have not decided what we should do.

https://github.com/QISKit/qiskit-tutorial/blob/master/reference/tools/quantum_gates_and_linear_algebra.ipynb

i am leaning towards deleting this gate and making a new one that we call cU which is a four parameter gate that has the three phases in cU3 and a relative phase. I would also like to change the name of cU1(theta) to cphase(theta)

rraymondhp commented 6 years ago

I am aware of the difference of u3 in the formula with what we have.

I wrote a note emphasizing the importance of global phase in control-U and its difference with just U. But that part is now missing. I will try to recover that part.

nelimee commented 6 years ago

Okey so the c-U3 is false in the current implementation. I corrected the phase shift with a c-RZZ:

from sympy import pi
def crzz(circuit, theta, ctrl, target):
    circuit.cu1(theta, ctrl, target)
    circuit.cx(ctrl, target)
    circuit.cu1(theta, ctrl, target)
    circuit.cx(ctrl, target)

def crx(circuit, theta, ctrl, target):
    # Apply the supposed c-RX operation.
    circuit.cu3(theta, pi/2, 3*pi/2, ctrl, target)
    # For the moment, QISKit adds a phase to the U-gate, so we
    # need to correct this phase with a controlled Rzz.
    crzz(circuit, pi, ctrl, target)

About the representation of quantum gates, I think there should be a huge warning somewhere that the representation used by QISKit is not the one used in the literature. Moreover, I think that a warning should be present in each method/function that expose the matrix representation of a gate. For the moment I found:

  1. The unitary simulator and the get_unitary method.
  2. The qiskit.mapper._compiling.two_qubit_kak method. For this one, the user does not know which representation is expected, whether the CX(0, 1) = [[1, 0, 0, 0], [0, 0, 0, 1], [0, 0, 1, 0], [0, 1, 0, 0]] one or the CX(0, 1) = [[1, 0, 0, 0], [1, 0, 0, 0], [0, 0, 0, 1], [0, 1, 0, 0]].
jaygambetta commented 6 years ago

cU3 is ambiguous and i agree we need to make it clearer. A real cU should have 4 variables not 3. The tensor order has been described in many places (see the tutorials) and it will be clearer in the documentation when we redo it.

chriseclectic commented 5 years ago

Closed by #2755