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

QuantumCircuit.mcrz not correct #9202

Closed
AlexisRalli opened this issue Nov 24, 2022 · 5 comments · Fixed by #9836
Closed

QuantumCircuit.mcrz not correct #9202

AlexisRalli opened this issue Nov 24, 2022 · 5 comments · Fixed by #9836
Assignees
Labels
bug Something isn't working priority: medium
Milestone

Comments

@AlexisRalli
Copy link

AlexisRalli commented Nov 24, 2022

Environment

  • Qiskit Terra version: 0.22.2
  • Python version: 3.9.7
  • Operating system: Darwin

What is happening?

multicontrol Rz rotation not correctly being built - see example below.

(Also multicontrol Rz, Ry, Rx gates can all be implemented using fewer nonlocal (two) qubit gates... See solution below)

How can we reproduce the issue?

from qiskit.quantum_info import Operator
from qiskit import QuantumCircuit, transpile
import numpy as np
from qiskit.circuit.library import MCMT


###### two qubit case ####
angle = np.pi/3
### correct
crz_circuit = QuantumCircuit(2)
crz_circuit.crz(angle, 0,1)
crz_circuit_unitary = Operator(crz_circuit).data

### incorrect
multi_crz = QuantumCircuit(2)
multi_crz.mcrz(angle, [0],1)
multi_crz_unitary = Operator(multi_crz).data

## FALSE
print('single control correct:', np.allclose(crz_circuit_unitary,
                             multi_crz_unitary))

###### multi control case ####
ctrl_list = [0,2,5]
target= 4

# ## Correct
gate = QuantumCircuit(1)
gate.rz(angle, 0)
  
mc_gate = MCMT(gate,
              num_ctrl_qubits=len(ctrl_list),
              num_target_qubits=1).to_gate()


mc_circuit = QuantumCircuit(max([target, max(ctrl_list)])+1)
mc_circuit.append(mc_gate, [*ctrl_list, target])


# ## Incorrect
mcrz_multi = QuantumCircuit(max([target, max(ctrl_list)])+1)
mcrz_multi.mcrz(angle,ctrl_list,target)


print('multicontrol correct:  ', np.allclose(Operator(mc_circuit).data,
                                   Operator(mcrz_multi).data))

What should happen?

mcrz method in QuantumCircuit should implement the correct quantum circuit. Solution provided below.

Any suggestions?

This code fixes the problem and acutally reduces the depth of all multicontrol rotation Rz,Ry,Rz gates:

from qiskit import QuantumCircuit

def primitive_block(angle, n_ctrl, axis):
    """
    get primative gates to perform multicontrol rotation
    """
    primitive=QuantumCircuit(2)
    if axis=='x':
        primitive.rx(angle/(2**n_ctrl), 1); primitive.cz(0,1)
    elif axis=='y':
        primitive.ry(angle/(2**n_ctrl), 1); primitive.cx(0,1)
    elif axis=='z':
        primitive.rz(angle/(2**n_ctrl), 1); primitive.cx(0,1)
    else:
        raise ValueError('Unrecognised axis, must be one of x,y or z.')
    
    return primitive

def get_pattern(n_ctrl, pattern=[0]):
    """
    find control sequence to implement an n_control gate 
    """
    if n_ctrl == pattern[-1]+1:
        return pattern
    else:
        new_pattern = pattern*2
        new_pattern[-1]+=1
        return get_pattern(n_ctrl, new_pattern)
    
def custom_mcrtl_rot(angle, ctrl_list, target, axis='y'):
    """
    return circuit that performs an n-control: Rz, Ry, Rx rotation
    
    """
    assert axis in ['x','y','z'], f'can only rotated around x,y,z axis, not {axis}'
    
    ctrl_list=sorted(ctrl_list)
    n_ctrl=len(ctrl_list)
    circ=QuantumCircuit(max([target, max(ctrl_list)])+1)    
    pattern = get_pattern(n_ctrl)
    
    if n_ctrl >1: 
        for s,i in enumerate(pattern):
            j = ctrl_list[-i-1]
            p = primitive_block((-1)**s*angle, n_ctrl, axis)
            circ = circ.compose(p, [j,target])

        circ = circ.compose(circ)
    else:
        p_plus = primitive_block(+angle, n_ctrl, axis)
        p_minus = primitive_block(-angle, n_ctrl, axis)
        circ = p_plus.compose(p_minus)
    
    return circ

We can check this is working via:

from qiskit.quantum_info import Operator
from qiskit import QuantumCircuit, transpile
import numpy as np
from qiskit.circuit.library import MCMT

### CAN CHANGE HERE
ctrl_list = [0,2,5]
target= 4
angle = np.pi/3
####


axis = 'z'
new_circuit = custom_mcrtl_rot(angle,ctrl_list,target, axis=axis)

gate = QuantumCircuit(1)
gate.rz(angle, 0)
mc_gate = MCMT(gate,
              num_ctrl_qubits=len(ctrl_list),
              num_target_qubits=1).to_gate()


mc_circuit = QuantumCircuit(max([target, max(ctrl_list)])+1)
mc_circuit.append(mc_gate, [*ctrl_list, target])

print('does new method match MCMT:', np.allclose(Operator(new_circuit).data,
            Operator(mc_circuit).data))


###  compare to mcrz
mcrz_multi = QuantumCircuit(max([target, max(ctrl_list)])+1)
mcrz_multi.mcrz(angle,ctrl_list,target)

print('does mcrz match MCMT:      ', np.allclose(Operator(mcrz_multi).data,
            Operator(new_circuit).data))

print('does mcrz match new:       ', np.allclose(Operator(mcrz_multi).data,
            Operator(mc_circuit).data))

### compare number of nonlocal gates in each approach depth
new_trans = transpile(new_circuit, basis_gates=['cx', 'rz','ry','rx'])
mctc_trans = transpile(mc_circuit, basis_gates=['cx', 'rz','ry','rx'])
mcrz_multi_trans = transpile(mcrz_multi, basis_gates=['cx', 'rz','ry','rx'])

print('\ndepth comparison')
print('num nonlocal gates new :', new_trans.num_nonlocal_gates())
print('num nonlocal gates mctc:', mctc_trans.num_nonlocal_gates())
print('num nonlocal gates mcrz:', mcrz_multi_trans.num_nonlocal_gates())

### allow old implementations to be optmized (NEW method not)
opt_mctc_trans = transpile(mc_circuit, optimization_level=3)
opt_mcrz_multi_trans = transpile(mcrz_multi, optimization_level=3)

print('\ndepth comparison (allowing old methods to be optimized)')
print('num nonlocal gates new :', new_trans.num_nonlocal_gates(), '(no optimization)')
print('num nonlocal gates mctc:', opt_mctc_trans.num_nonlocal_gates())
print('num nonlocal gates mcrz:', opt_mcrz_multi_trans.num_nonlocal_gates())


### prints:

# does new method match MCMT: True
# does mcrz match MCMT:       False
# does mcrz match new:        False

# depth comparison
# num nonlocal gates new : 8
# num nonlocal gates mctc: 28
# num nonlocal gates mcrz: 20

# depth comparison (allowing old methods to be optimized)
# num nonlocal gates new : 8 (no optimization)
# num nonlocal gates mctc: 25
# num nonlocal gates mcrz: 13


The new approach gives an improvement to the correctly implemented mutlicontrol rotation gates currently coded in qiskit:

### CAN CHANGE HERE
ctrl_list = [0,2,5]
target= 4
angle = np.pi/3
####


for ax in ['x','y']:
    
    new_circuit = custom_mcrtl_rot(angle,ctrl_list,target, axis=ax)

    if ax =='x':
        gate = QuantumCircuit(1)
        gate.rx(angle, 0) #< --- rx
    elif ax == 'y':
        gate = QuantumCircuit(1)
        gate.ry(angle, 0) #< --- ry
        
    mc_gate = MCMT(gate,
                  num_ctrl_qubits=len(ctrl_list),
                  num_target_qubits=1).to_gate()

    mc_circuit = QuantumCircuit(max([target, max(ctrl_list)])+1)
    mc_circuit.append(mc_gate, [*ctrl_list, target])

    print('does new method match MCMT:', np.allclose(Operator(new_circuit).data,
                                         Operator(mc_circuit).data))
    print()


    ###  compare to mcr
    mcr_multi = QuantumCircuit(max([target, max(ctrl_list)])+1)
    if ax =='x':
        mcr_multi.mcrx(angle,ctrl_list,target)
    elif ax == 'y':
        mcr_multi.mcry(angle,ctrl_list,target)
    

    print(f'does mcr{ax} match MCMT:      ', np.allclose(Operator(mcr_multi).data,
                                             Operator(new_circuit).data))

    print(f'does mcr{ax} match new:       ', np.allclose(Operator(mcr_multi).data,
                                            Operator(mc_circuit).data))

    ### compare number of nonlocal gates in each approach depth
    new_trans = transpile(new_circuit, basis_gates=['cx', 'rz','ry','rx'])
    mctc_trans = transpile(mc_circuit, basis_gates=['cx', 'rz','ry','rx'])
    mcr_multi_trans = transpile(mcr_multi, basis_gates=['cx', 'rz','ry','rx'])

    print(f'\ndepth comparison for multi control R{ax} gate')
    print('num nonlocal gates new :', new_trans.num_nonlocal_gates())
    print('num nonlocal gates mctc:', mctc_trans.num_nonlocal_gates())
    print(f'num nonlocal gates mcr{ax}:', mcr_multi_trans.num_nonlocal_gates())

    ### allow old implementations to be optmized (NEW method not)
    opt_mctc_trans = transpile(mc_circuit, optimization_level=3)
    opt_mcrX_multi_trans = transpile(mcr_multi, optimization_level=3)
    
    print(f'\ndepth comparison for multi control R{ax} gate (allowing old methods to be optimized)')
    print('num nonlocal gates new :', new_trans.num_nonlocal_gates(), '(no optimization)')
    print('num nonlocal gates mctc:', opt_mctc_trans.num_nonlocal_gates())
    print(f'num nonlocal gates mcr{ax}:', opt_mcrX_multi_trans.num_nonlocal_gates())

    
    print('*'*12)
    
# does new method match MCMT: True

# does mcrx match MCMT:       True
# does mcrx match new:        True

# depth comparison for multi control Rx gate
# num nonlocal gates new : 8
# num nonlocal gates mctc: 20
# num nonlocal gates mcrx: 20

# depth comparison for multi control Rx gate (allowing old methods to be optimized)
# num nonlocal gates new : 8 (no optimization)
# num nonlocal gates mctc: 20
# num nonlocal gates mcrx: 13
# ************
# does new method match MCMT: True

# does mcry match MCMT:       True
# does mcry match new:        True

# depth comparison for multi control Ry gate
# num nonlocal gates new : 8
# num nonlocal gates mctc: 20
# num nonlocal gates mcry: 20

# depth comparison for multi control Ry gate (allowing old methods to be optimized)
# num nonlocal gates new : 8 (no optimization)
# num nonlocal gates mctc: 20
# num nonlocal gates mcry: 13
# ************

@AlexisRalli AlexisRalli added the bug Something isn't working label Nov 24, 2022
@AlexisRalli AlexisRalli changed the title QuantumCircuit.mcrz not building correct quantum circuit QuantumCircuit.mcrz not correct Dec 2, 2022
@Cryoris
Copy link
Contributor

Cryoris commented Dec 13, 2022

Hi @AlexisRalli, thanks for reporting this! Indeed the MCRZ looks wrong, it's not equivalent to the multicontrolled phase gates, as which it is implemented. Do you have a reference to the multicontrolled Pauli rotations you're using? We could definitely add those as default decomposition to Qiskit if they use less CNOTS (or have shallower depth) 🙂

@AlexisRalli
Copy link
Author

Hi @Cryoris , I was using Theorem 8 of https://arxiv.org/pdf/quant-ph/0406176.pdf to build these circuits.

@Cryoris
Copy link
Contributor

Cryoris commented Jan 11, 2023

Thanks for the reference! Would you be interested in fixing this in Qiskit? 🙂 We can of course have a discussion on how to tackle this, if that would be helpful.

@Cryoris Cryoris added this to the 0.24.0 milestone Jan 11, 2023
@AlexisRalli
Copy link
Author

@Cryoris Sorry for the late reply. I'd be happy to fix this. I guess I should just fork the repo and make a pull request

@HuangJunye
Copy link
Collaborator

@AlexisRalli I have assigned you the issue. You can find instructions on how to setup your local development environment in the contributing guide. If you need any help, please let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: medium
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants