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.

Sign up for GitHub

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

Jump to bottom

Fix wrong relative phase of MCRZ #9836

Merged
merged 46 commits into from
May 22, 2023
Merged

Fix wrong relative phase of MCRZ #9836

Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
9f49257
efficient multicontrolled su2 gate decomposition
rafaella-vale Mar 1, 2023
2c81328
removed optimization flag
rafaella-vale Mar 1, 2023
a8fbb81
tox -eblack
adjs Mar 2, 2023
5151a42
updated docstrings
rafaella-vale Mar 3, 2023
0aec4ec
Adds `MCSU2Gate` to `__init__`
israelferrazaraujo Mar 3, 2023
b01a093
Merge branch 'mcsu2' of https://github.com/comp-quantica/qiskit-terra…
rafaella-vale Mar 3, 2023
e39e09d
fixed circular import
rafaella-vale Mar 3, 2023
d632819
defined control and inverse methods
rafaella-vale Mar 3, 2023
32acacc
changed MCSU2Gate from Gate to ControlledGate
rafaella-vale Mar 4, 2023
3c8bdc4
adjusted some tests for controlled gates
rafaella-vale Mar 4, 2023
3a92dc9
reformatting
rafaella-vale Mar 4, 2023
155a982
Merge branch 'main' into mcsu2
rafaella-vale Mar 4, 2023
ed6953b
Fix regarding the integer `ctrl_state` parameter
israelferrazaraujo Mar 5, 2023
100a690
Tests to check the CX count upper bound
israelferrazaraujo Mar 5, 2023
7d3ea14
Gate's `label` in the `control` function
israelferrazaraujo Mar 5, 2023
7c75611
Upd. Qiskit tests to include cases for MCSU2Gate
israelferrazaraujo Mar 5, 2023
c1ceaf6
Upd. Qiskit tests to include cases for MCSU2Gate
israelferrazaraujo Mar 5, 2023
39d35b4
Revert "Upd. Qiskit tests to include cases for MCSU2Gate"
adjs Mar 5, 2023
c00d17a
Revert "Upd. Qiskit tests to include cases for MCSU2Gate"
adjs Mar 5, 2023
0a6148d
Revert "Tests to check the CX count upper bound"
adjs Mar 5, 2023
f4042c1
Update test_controlled_gate.py
adjs Mar 6, 2023
b999c20
Update test_circuit_operations.py
adjs Mar 6, 2023
0c3eeed
remove mcsu2gate class
adjs Mar 6, 2023
6bdb533
remove mcsu2gate class
adjs Mar 6, 2023
9b02451
fix mcry
adjs Mar 6, 2023
2568a39
lint
adjs Mar 6, 2023
15034fd
fix mcrx
adjs Mar 7, 2023
fe874ef
add reference
adjs Mar 8, 2023
b762b39
Create `s_gate` directly
israelferrazaraujo Mar 14, 2023
d75b9c9
Revert "Create `s_gate` directly"
adjs Mar 14, 2023
d584340
review
adjs Mar 14, 2023
1fe9ce5
release notes
adjs Mar 15, 2023
de3d840
review 2
adjs Mar 15, 2023
e39de45
backwards compat
adjs Mar 15, 2023
4e4f14d
function signature and number of controls
adjs Mar 17, 2023
ac25805
fix mcrz
adjs Mar 22, 2023
ab9920f
Update multi_control_rotation_gates.py
adjs Mar 22, 2023
b94e936
Merge branch 'main' into fix-mcrz
adjs Apr 19, 2023
17ce419
review
adjs Apr 21, 2023
fab1c80
Update test_qpy.py
adjs Apr 21, 2023
3aedea6
Revert "Update test_qpy.py"
adjs Apr 21, 2023
fd14c87
Update test_qpy.py
adjs May 3, 2023
856cd15
Merge branch 'main' into fix-mcrz
adjs May 3, 2023
7a9df50
Update multi_control_rotation_gates.py
adjs May 4, 2023
9269ecf
Fix `use_basis_gates=True` case
Cryoris May 16, 2023
fb180f4
lint
Cryoris May 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions qiskit/circuit/add_control.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ def control(
q_target[bit_indices[qargs[0]]],
use_basis_gates=True,
)
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because the decomposition is phase correct and the fragment below will change the global phase.

            if gate.definition is not None and gate.definition.global_phase:
                global_phase += gate.definition.global_phase

elif gate.name == "p":
from qiskit.circuit.library import MCPhaseGate

Expand Down Expand Up @@ -184,23 +185,17 @@ def control(
use_basis_gates=True,
)
elif theta == 0 and phi == 0:
controlled_circ.mcrz(
lamb, q_control, q_target[bit_indices[qargs[0]]], use_basis_gates=True
)
controlled_circ.mcp(lamb, q_control, q_target[bit_indices[qargs[0]]])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this output still the same as before, without the explicit use_basis_gates and q_ancillae? 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still has no auxiliary qubits, however the basis_gates are different. This is an internal change and probably doesn't need to be reported in the release notes.

else:
controlled_circ.mcrz(
lamb, q_control, q_target[bit_indices[qargs[0]]], use_basis_gates=True
)
controlled_circ.mcp(lamb, q_control, q_target[bit_indices[qargs[0]]])
controlled_circ.mcry(
theta,
q_control,
q_target[bit_indices[qargs[0]]],
q_ancillae,
use_basis_gates=True,
)
controlled_circ.mcrz(
phi, q_control, q_target[bit_indices[qargs[0]]], use_basis_gates=True
)
controlled_circ.mcp(phi, q_control, q_target[bit_indices[qargs[0]]])
elif gate.name == "z":
controlled_circ.h(q_target[bit_indices[qargs[0]]])
controlled_circ.mcx(q_control, q_target[bit_indices[qargs[0]]], q_ancillae)
Expand Down
49 changes: 29 additions & 20 deletions qiskit/circuit/library/standard_gates/multi_control_rotation_gates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,28 +84,28 @@ def _apply_mcu_graycode(circuit, theta, phi, lam, ctls, tgt, use_basis_gates):


def mcsu2_real_diagonal(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to mark this function as private as it is a utility and not API documented. If we want to expose it, it might be worth to turn it into it's own class in a follow-up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept it public, but we can easily make its private and delete the release note of #9688.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better if it's private, this way we can still move it e.g. to the synthesis module later, or turn it into a class without having to deprecate it. Since the release is imminent, I'll open a short separate PR for that 🙂

circuit,
circuit: QuantumCircuit,
unitary: np.ndarray,
controls: Union[QuantumRegister, List[Qubit]],
target: Union[Qubit, int],
ctrl_state: str = None,
ctrl_state: Optional[str] = None,
):
"""
Apply multi-controlled SU(2) gate with a real main diagonal or secondary diagonal.
https://arxiv.org/abs/2302.06377

Args:
circuit (QuantumCircuit): The QuantumCircuit object to apply the diagonal operator on.
unitary (ndarray): SU(2) unitary matrix with one real diagonal
controls (QuantumRegister or list(Qubit)): The list of control qubits
target (Qubit or int): The target qubit
ctrl_state (str): control state of the operator SU(2) operator
circuit: The QuantumCircuit object to apply the diagonal operator on.
unitary: SU(2) unitary matrix with one real diagonal
controls: The list of control qubits
target: The target qubit
ctrl_state: control state of the operator SU(2) operator

Raises:
QiskitError: parameter errors
"""
# pylint: disable=cyclic-import
from qiskit.circuit.library import MCXVChain
from .x import MCXVChain
from qiskit.extensions import UnitaryGate
from qiskit.quantum_info.operators.predicates import is_unitary_matrix

Expand All @@ -130,13 +130,17 @@ def mcsu2_real_diagonal(
x = -unitary[0, 1].real
z = unitary[1, 1] - unitary[0, 1].imag * 1.0j

alpha_r = np.sqrt((np.sqrt((z.real + 1.0) / 2.0) + 1.0) / 2.0)
alpha_i = z.imag / (2.0 * np.sqrt((z.real + 1.0) * (np.sqrt((z.real + 1.0) / 2.0) + 1.0)))
alpha = alpha_r + 1.0j * alpha_i
beta = x / (2.0 * np.sqrt((z.real + 1.0) * (np.sqrt((z.real + 1.0) / 2.0) + 1.0)))
if np.isclose(z, -1):
s_op = [[1.0, 0.0], [0.0, 1.0j]]
else:
alpha_r = np.sqrt((np.sqrt((z.real + 1.0) / 2.0) + 1.0) / 2.0)
alpha_i = z.imag / (2.0 * np.sqrt((z.real + 1.0) * (np.sqrt((z.real + 1.0) / 2.0) + 1.0)))
alpha = alpha_r + 1.0j * alpha_i
beta = x / (2.0 * np.sqrt((z.real + 1.0) * (np.sqrt((z.real + 1.0) / 2.0) + 1.0)))

# S gate definition
s_op = np.array([[alpha, -np.conj(beta)], [beta, np.conj(alpha)]])

# S gate definition
s_op = np.array([[alpha, -np.conj(beta)], [beta, np.conj(alpha)]])
s_gate = UnitaryGate(s_op)

num_ctrl = len(controls)
Expand Down Expand Up @@ -327,6 +331,8 @@ def mcrz(
Raises:
QiskitError: parameter errors
"""
from .rz import CRZGate, RZGate

control_qubits = self.qbit_argument_conversion(q_controls)
target_qubit = self.qbit_argument_conversion(q_target)
if len(target_qubit) != 1:
Expand All @@ -336,13 +342,16 @@ def mcrz(
self._check_dups(all_qubits)

n_c = len(control_qubits)
if n_c == 1: # cu
_apply_cu(self, 0, 0, lam, control_qubits[0], target_qubit, use_basis_gates=use_basis_gates)
if n_c == 1:
self.append(CRZGate(lam), control_qubits + [target_qubit])
else:
lam_step = lam * (1 / (2 ** (n_c - 1)))
_apply_mcu_graycode(
self, 0, 0, lam_step, control_qubits, target_qubit, use_basis_gates=use_basis_gates
)
mcsu2_real_diagonal(self, RZGate(lam).to_matrix(), control_qubits, target_qubit)

if use_basis_gates:
# pylint: disable=cyclic-import
from qiskit import transpile

self = transpile(self, basis_gates=["p", "u", "cx"], optimization_level=0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is quite correct, sorry if I didn't express it correctly before. With this code, the whole circuit will get transpiled to the specified basis -- but we only want the newly added mcrz to be in that basis 🙂 maybe we can just change mcsu2_real_diagonal to return the MCRZ subcircuit and then transpile that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Cryoris , I removed the transpile call.
I think that we should not transpile the MCRZ circuit here, because of the transpile cost with a higher number of controls.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's not the most efficient way, but we should ensure that use_basis_gates=True will still behave as it did before 🙂 If we want to avoid transpiling, would it be possible to express mcsu2 in the P U CX basis?



QuantumCircuit.mcrx = mcrx
Expand Down
7 changes: 7 additions & 0 deletions releasenotes/notes/fix-mcrz-relative-phase-6ea81a369f8bda38.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
fixes:
- |
Fixed the gate decomposition of multi-controlled Z rotation gates added via
:meth:`.QuantumCircuit.mcrz`. Previously, this method implemented a multi-controlled
phase gate, which has a relative phase difference to the Z rotation. To obtain the
previous `.QuantumCircuit.mcrz` behaviour, use `.QuantumCircuit.mcp`.
2 changes: 1 addition & 1 deletion test/python/circuit/test_controlled_gate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ def test_multi_controlled_rotation_gate_matrices(
elif base_gate_name == "y":
rot_mat = RYGate(theta).to_matrix()
else: # case 'z'
rot_mat = U1Gate(theta).to_matrix()
rot_mat = RZGate(theta).to_matrix()

expected = _compute_control_matrix(rot_mat, num_controls, ctrl_state=ctrl_state)
with self.subTest(msg=f"control state = {ctrl_state}"):
Expand Down