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

Improved MCXVChain with dirty auxiliary qubits #9687

Merged
merged 16 commits into from
Jul 7, 2024
Merged

Improved MCXVChain with dirty auxiliary qubits #9687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
087d986
improved mcxvchain with dirty aux qubits
rafaella-vale Feb 23, 2023
5ec3c55
tests for improved mcxvchain with dirty aux qubits
rafaella-vale Feb 27, 2023
bd76eeb
black
adjs Mar 2, 2023
5f2c6f7
lint
adjs Mar 2, 2023
f5e2c3a
Update x.py
adjs Mar 3, 2023
ab6a4c4
Update test_controlled_gate.py
adjs Mar 3, 2023
41d0db9
Update test_controlled_gate.py
adjs Mar 3, 2023
e7ed2c1
release notes
adjs Mar 4, 2023
e460a83
removing non existend module
IsmaelCesar Mar 13, 2024
13fd6b0
removing U1 and U2 gates MCXVChain
IsmaelCesar Mar 13, 2024
013c8a5
black
IsmaelCesar Mar 13, 2024
0480af4
using the quantum_circuit.operation(qubit_indices) notation
IsmaelCesar Mar 15, 2024
0f1c253
mcx
IsmaelCesar Mar 15, 2024
e61eda3
comments
IsmaelCesar Apr 2, 2024
17da83e
lint
IsmaelCesar Jun 27, 2024
95264ed
rearranging transpile import
IsmaelCesar Jun 27, 2024
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
151 changes: 96 additions & 55 deletions qiskit/circuit/library/standard_gates/x.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1409,6 +1409,8 @@ def __new__(
duration=None,
unit="dt",
_base_label=None,
relative_phase: bool = False, # pylint: disable=unused-argument
action_only: bool = False, # pylint: disable=unused-argument
):
"""Create a new MCX instance.

Expand All @@ -1434,7 +1436,24 @@ def __init__(
duration=None,
unit="dt",
_base_label=None,
relative_phase: bool = False,
action_only: bool = False,
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add proper documentation on all of these parameters so that the user will know what they are doing and what to choose?
It will be good to extend the docstring of the MCXVChain class, add a reference to the relevant paper and theorem.
Perhaps even some examples if possible?

Copy link
Member

Choose a reason for hiding this comment

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

It seems that there are 3 boolean parameters:
dirty_ancillas, relative_phase, action_only
Could we have all 8 options for these parameters? or only part of the options are possible?
If so, please add it to the docs, check and raise a QiskitError if the user chose a bad option.
Otherwise, please add tests for all relevant options.

Copy link
Member

Choose a reason for hiding this comment

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

Are there some bounds on the number of CX gates for these options?
If so, could you please add this to the docs, as well as add proper tests.

):
"""
Args:
dirty_ancillas: when set to ``True``, the method applies an optimized multicontrolled-X gate
up to a relative phase using dirty ancillary qubits with the properties of lemmas 7 and 8
from arXiv:1501.06911, with at most 8*k - 6 CNOT gates.
For k within the range {1, ..., ceil(n/2)}. And for n representing the total number of
qubits.
relative_phase: when set to ``True``, the method applies the optimized multicontrolled-X gate
up to a relative phase, in a way that, by lemma 7 of arXiv:1501.06911, the relative
phases of the ``action part`` cancel out with the phases of the ``reset part``.

action_only: when set to ``True``, the method applies only the action part of lemma 8
from arXiv:1501.06911.

"""
super().__init__(
num_ctrl_qubits,
label=label,
Expand All @@ -1445,6 +1464,9 @@ def __init__(
unit=unit,
)
self._dirty_ancillas = dirty_ancillas
self._relative_phase = relative_phase
self._action_only = action_only
super().__init__(num_ctrl_qubits, label=label, ctrl_state=ctrl_state, _name="mcx_vchain")

def inverse(self, annotated: bool = False):
"""Invert this gate. The MCX is its own inverse.
Expand All @@ -1462,6 +1484,8 @@ def inverse(self, annotated: bool = False):
num_ctrl_qubits=self.num_ctrl_qubits,
dirty_ancillas=self._dirty_ancillas,
ctrl_state=self.ctrl_state,
relative_phase=self._relative_phase,
Cryoris marked this conversation as resolved.
Show resolved Hide resolved
action_only=self._action_only,
)

@staticmethod
Expand All @@ -1473,73 +1497,90 @@ def _define(self):
"""Define the MCX gate using a V-chain of CX gates."""
# pylint: disable=cyclic-import
from qiskit.circuit.quantumcircuit import QuantumCircuit
from .u1 import U1Gate
from .u2 import U2Gate

q = QuantumRegister(self.num_qubits, name="q")
qc = QuantumCircuit(q, name=self.name)
q_controls = q[: self.num_ctrl_qubits]
q_target = q[self.num_ctrl_qubits]
q_ancillas = q[self.num_ctrl_qubits + 1 :]

definition = []

if self._dirty_ancillas:
i = self.num_ctrl_qubits - 3
ancilla_pre_rule = [
(U2Gate(0, numpy.pi), [q_target], []),
(CXGate(), [q_target, q_ancillas[i]], []),
(U1Gate(-numpy.pi / 4), [q_ancillas[i]], []),
(CXGate(), [q_controls[-1], q_ancillas[i]], []),
(U1Gate(numpy.pi / 4), [q_ancillas[i]], []),
(CXGate(), [q_target, q_ancillas[i]], []),
(U1Gate(-numpy.pi / 4), [q_ancillas[i]], []),
(CXGate(), [q_controls[-1], q_ancillas[i]], []),
(U1Gate(numpy.pi / 4), [q_ancillas[i]], []),
]
for inst in ancilla_pre_rule:
definition.append(inst)
if self.num_ctrl_qubits < 3:
qc.mcx(q_controls, q_target)
elif not self._relative_phase and self.num_ctrl_qubits == 3:
qc._append(C3XGate(), [*q_controls, q_target], [])
else:
num_ancillas = self.num_ctrl_qubits - 2
Copy link
Contributor

Choose a reason for hiding this comment

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

To make this code a little easier to read and make it easier to potentially move the code to qiskit.synthesis at a later stage, could you put this synthesis technique into a private function (could be in this file) and then just call that function here? 🙂

Copy link
Member

@ShellyGarion ShellyGarion Mar 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In addition, it would be useful to keep the current synthesis code in another internal function.
Namely, to put the current synthesis code in the definition in a function called _synth_MCXVChain_basic and the new code in _synth_MCXVChain_isometry. Both functions should return a QuantumCircuit. The definition will call the new synthesis function.

Copy link
Contributor

Choose a reason for hiding this comment

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

When calling the private function on the qiskit.synthesis we must create a new file in a new directory?

Copy link
Member

@ShellyGarion ShellyGarion Mar 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In the meanwhile, you don't need to move the code to qiskit.synthesis. Just organize it in internal functions as we suggested above (in this file), so that later it will be easier for us to move it if needed.

Copy link
Member

Choose a reason for hiding this comment

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

The idea is that not all this code will be inside the define function.
You can put the existing code inside _synth_MCXVChain_basic and the new code in _synth_MCXVChain_isometry,
and let the define function call the new _synth_MCXVChain_isometry.

targets = [q_target] + q_ancillas[:num_ancillas][::-1]

for j in range(2):
for i in range(self.num_ctrl_qubits): # action part
if i < self.num_ctrl_qubits - 2:
if targets[i] != q_target or self._relative_phase:
# gate cancelling

# cancel rightmost gates of action part
# with leftmost gates of reset part
if self._relative_phase and targets[i] == q_target and j == 1:
qc.cx(q_ancillas[num_ancillas - i - 1], targets[i])
qc.t(targets[i])
qc.cx(q_controls[self.num_ctrl_qubits - i - 1], targets[i])
qc.tdg(targets[i])
qc.h(targets[i])
else:
Copy link
Member

Choose a reason for hiding this comment

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

This code looks much clearer now, but it seems that it contains many if/else clauses.
Could you please add a comment after each if/else/elif and to say which case it handles?

qc.h(targets[i])
qc.t(targets[i])
qc.cx(q_controls[self.num_ctrl_qubits - i - 1], targets[i])
qc.tdg(targets[i])
qc.cx(q_ancillas[num_ancillas - i - 1], targets[i])
else:
controls = [
q_controls[self.num_ctrl_qubits - i - 1],
q_ancillas[num_ancillas - i - 1],
]

qc.ccx(controls[0], controls[1], targets[i])
else:
# implements an optimized toffoli operation
# up to a diagonal gate, akin to lemma 6 of arXiv:1501.06911
qc.h(targets[i])
qc.t(targets[i])
qc.cx(q_controls[self.num_ctrl_qubits - i - 2], targets[i])
qc.tdg(targets[i])
qc.cx(q_controls[self.num_ctrl_qubits - i - 1], targets[i])
qc.t(targets[i])
qc.cx(q_controls[self.num_ctrl_qubits - i - 2], targets[i])
qc.tdg(targets[i])
qc.h(targets[i])

break

for i in range(num_ancillas - 1): # reset part
qc.cx(q_ancillas[i], q_ancillas[i + 1])
qc.t(q_ancillas[i + 1])
qc.cx(q_controls[2 + i], q_ancillas[i + 1])
qc.tdg(q_ancillas[i + 1])
qc.h(q_ancillas[i + 1])

if self._action_only:
qc.ccx(q_controls[-1], q_ancillas[-1], q_target)

break
else:
qc.rccx(q_controls[0], q_controls[1], q_ancillas[0])
i = 0
for j in range(2, self.num_ctrl_qubits - 1):
qc.rccx(q_controls[j], q_ancillas[i], q_ancillas[i + 1])

for j in reversed(range(2, self.num_ctrl_qubits - 1)):
definition.append(
(RCCXGate(), [q_controls[j], q_ancillas[i - 1], q_ancillas[i]], [])
)
i -= 1
i += 1

definition.append((RCCXGate(), [q_controls[0], q_controls[1], q_ancillas[0]], []))
i = 0
for j in range(2, self.num_ctrl_qubits - 1):
definition.append((RCCXGate(), [q_controls[j], q_ancillas[i], q_ancillas[i + 1]], []))
i += 1
qc.ccx(q_controls[-1], q_ancillas[i], q_target)

if self._dirty_ancillas:
ancilla_post_rule = [
(U1Gate(-numpy.pi / 4), [q_ancillas[i]], []),
(CXGate(), [q_controls[-1], q_ancillas[i]], []),
(U1Gate(numpy.pi / 4), [q_ancillas[i]], []),
(CXGate(), [q_target, q_ancillas[i]], []),
(U1Gate(-numpy.pi / 4), [q_ancillas[i]], []),
(CXGate(), [q_controls[-1], q_ancillas[i]], []),
(U1Gate(numpy.pi / 4), [q_ancillas[i]], []),
(CXGate(), [q_target, q_ancillas[i]], []),
(U2Gate(0, numpy.pi), [q_target], []),
]
for inst in ancilla_post_rule:
definition.append(inst)
else:
definition.append((CCXGate(), [q_controls[-1], q_ancillas[i], q_target], []))
for j in reversed(range(2, self.num_ctrl_qubits - 1)):
qc.rccx(q_controls[j], q_ancillas[i - 1], q_ancillas[i])

for j in reversed(range(2, self.num_ctrl_qubits - 1)):
definition.append((RCCXGate(), [q_controls[j], q_ancillas[i - 1], q_ancillas[i]], []))
i -= 1
definition.append((RCCXGate(), [q_controls[0], q_controls[1], q_ancillas[i]], []))
i -= 1

if self._dirty_ancillas:
for i, j in enumerate(list(range(2, self.num_ctrl_qubits - 1))):
definition.append(
(RCCXGate(), [q_controls[j], q_ancillas[i], q_ancillas[i + 1]], [])
)
qc.rccx(q_controls[0], q_controls[1], q_ancillas[i])

for instr, qargs, cargs in definition:
qc._append(instr, qargs, cargs)
self.definition = qc
10 changes: 10 additions & 0 deletions releasenotes/notes/mcxvchain_dirty_auxiliary-5ea4037557209f6e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
features:
- |
:class:`.MCXVChain` has two new Boolean parameters `relative_phase` and `action_only`.
If `action_only` the circuit does not clean the dirty qubits. If `relative_phase`
the gate is implemented up to a global phase. Both parameters are used to optimize the
decomposition of MCXVChain.
fixes:
- |
:class:`.MCXVChain` with k controls and k-2 dirty auxiliary qubits now requires 8k-6 cx gates.
61 changes: 61 additions & 0 deletions test/python/circuit/test_controlled_gate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,67 @@ def test_mcx_gates(self, num_ctrl_qubits):
statevector = corrected
np.testing.assert_array_almost_equal(statevector.real, reference)

@data(5, 10, 15)
def test_mcxvchain_dirty_ancilla_cx_count(self, num_ctrl_qubits):
"""Test if cx count of the v-chain mcx with dirty ancilla
is less than upper bound."""
from qiskit import transpile

mcx_vchain = MCXVChain(num_ctrl_qubits, dirty_ancillas=True)
qc = QuantumCircuit(mcx_vchain.num_qubits)

qc.append(mcx_vchain, list(range(mcx_vchain.num_qubits)))

tr_mcx_vchain = transpile(qc, basis_gates=["u", "cx"])
cx_count = tr_mcx_vchain.count_ops()["cx"]

self.assertLessEqual(cx_count, 8 * num_ctrl_qubits - 6)

def test_mcxvchain_dirty_ancilla_action_only(self):
"""Test the v-chain mcx with dirty auxiliary qubits
with gate cancelling with mirrored circuit."""

num_ctrl_qubits = 5

gate = MCXVChain(num_ctrl_qubits, dirty_ancillas=True)
gate_with_cancelling = MCXVChain(num_ctrl_qubits, dirty_ancillas=True, action_only=True)

num_qubits = gate.num_qubits
ref_circuit = QuantumCircuit(num_qubits)
circuit = QuantumCircuit(num_qubits)

ref_circuit.append(gate, list(range(num_qubits)), [])
ref_circuit.h(num_ctrl_qubits)
ref_circuit.append(gate, list(range(num_qubits)), [])

circuit.append(gate_with_cancelling, list(range(num_qubits)), [])
circuit.h(num_ctrl_qubits)
circuit.append(gate_with_cancelling.inverse(), list(range(num_qubits)), [])

self.assertTrue(matrix_equal(Operator(circuit).data, Operator(ref_circuit).data))

def test_mcxvchain_dirty_ancilla_relative_phase(self):
"""Test the v-chain mcx with dirty auxiliary qubits
with only relative phase Toffoli gates."""
num_ctrl_qubits = 5

gate = MCXVChain(num_ctrl_qubits, dirty_ancillas=True)
gate_relative_phase = MCXVChain(num_ctrl_qubits, dirty_ancillas=True, relative_phase=True)

num_qubits = gate.num_qubits + 1
ref_circuit = QuantumCircuit(num_qubits)
circuit = QuantumCircuit(num_qubits)

ref_circuit.append(gate, list(range(num_qubits - 1)), [])
ref_circuit.h(num_qubits - 1)
ref_circuit.append(gate, list(range(num_qubits - 1)), [])

circuit.append(gate_relative_phase, list(range(num_qubits - 1)), [])
circuit.h(num_qubits - 1)
circuit.append(gate_relative_phase.inverse(), list(range(num_qubits - 1)), [])

self.assertTrue(matrix_equal(Operator(circuit).data, Operator(ref_circuit).data))

@data(1, 2, 3, 4)
def test_inverse_x(self, num_ctrl_qubits):
"""Test inverting the controlled X gate."""
Expand Down
Loading