Skip to content

Commit

Permalink
Remove cyclic-definition handling from Clifford.from_circuit (#10441)
Browse files Browse the repository at this point in the history
* Remove cyclic-definition handling from `Clifford.from_circuit`

This reliance on `RecursionError` could lead to CPython crashing, when
the user had raised the recursion limit beyond what their operating
system could actually support in terms of Python frames.  This was
particularly an issue with Windows when in a context that `jedi` is
active (such as in an IPython session, or if `seaborn` was imported),
since `jedi` unilaterally sets the recursion limit to 3000, while
CPython tends to overflow the stack on Windows at around 2700 frames.

Recursive `definition` fields are not valid data in the Qiskit model, as
the definition is supposed to be hierarchical, and a decomposition in
terms of gates that do not involve the current one in any form.  For
defining equivalences that may involve cycles, one should use the
`EquivalenceLibrary` objects that Terra manages, and the transpiler
takes advantage of via the `BasisTranslator`.

* Remove unused import
  • Loading branch information
jakelishman authored Jul 18, 2023
1 parent b831bcf commit 35f9e7c
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 49 deletions.
25 changes: 9 additions & 16 deletions qiskit/quantum_info/operators/symplectic/clifford_circuits.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,23 @@
"""
Circuit simulation for the Clifford class.
"""

from __future__ import annotations
import copy

import numpy as np

from qiskit.circuit import Barrier, Delay, Gate
from qiskit.circuit.exceptions import CircuitError
from qiskit.exceptions import QiskitError


def _append_circuit(clifford, circuit, qargs=None, recursion_depth=0):
def _append_circuit(clifford, circuit, qargs=None):
"""Update Clifford inplace by applying a Clifford circuit.
Args:
clifford (Clifford): The Clifford to update.
circuit (QuantumCircuit): The circuit to apply.
qargs (list or None): The qubits to apply circuit to.
recursion_depth (int): The depth of mutual recursion with _append_operation
Returns:
Clifford: the updated Clifford.
Expand All @@ -46,18 +46,17 @@ def _append_circuit(clifford, circuit, qargs=None, recursion_depth=0):
)
# Get the integer position of the flat register
new_qubits = [qargs[circuit.find_bit(bit).index] for bit in instruction.qubits]
clifford = _append_operation(clifford, instruction.operation, new_qubits, recursion_depth)
clifford = _append_operation(clifford, instruction.operation, new_qubits)
return clifford


def _append_operation(clifford, operation, qargs=None, recursion_depth=0):
def _append_operation(clifford, operation, qargs=None):
"""Update Clifford inplace by applying a Clifford operation.
Args:
clifford (Clifford): The Clifford to update.
operation (Instruction or Clifford or str): The operation or composite operation to apply.
qargs (list or None): The qubits to apply operation to.
recursion_depth (int): The depth of mutual recursion with _append_circuit
Returns:
Clifford: the updated Clifford.
Expand Down Expand Up @@ -135,16 +134,10 @@ def _append_operation(clifford, operation, qargs=None, recursion_depth=0):
# This succeeds only if the gate has all-Clifford definition (decomposition).
# If fails, we need to restore the clifford that was before attempting to unroll and append.
if gate.definition is not None:
if recursion_depth > 0:
return _append_circuit(clifford, gate.definition, qargs, recursion_depth + 1)
else: # recursion_depth == 0
# clifford may be updated in _append_circuit
org_clifford = copy.deepcopy(clifford)
try:
return _append_circuit(clifford, gate.definition, qargs, 1)
except (QiskitError, RecursionError):
# discard incompletely updated clifford and continue
clifford = org_clifford
try:
return _append_circuit(clifford.copy(), gate.definition, qargs)
except QiskitError:
pass

# As a final attempt, if the gate is up to 3 qubits,
# we try to construct a Clifford to be appended from its matrix representation.
Expand Down
9 changes: 9 additions & 0 deletions releasenotes/notes/clifford-no-circuly-c7c4a1c9c5472af7.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
upgrade:
- |
:meth:`.Clifford.from_circuit` will no longer attempt to resolve instructions whose
:attr:`~.circuit.Instruction.definition` fields are mutually recursive with some other object.
Such recursive definitions are already a violation of the strictly hierarchical ordering that
the :attr:`~.circuit.Instruction.definition` field requires, and code should not rely on this
being possible at all. If you want to define equivalences that are permitted to have (mutual)
cycles, use an :class:`.EquivalenceLibrary`.
33 changes: 0 additions & 33 deletions test/python/quantum_info/operators/symplectic/test_clifford.py
Original file line number Diff line number Diff line change
Expand Up @@ -555,39 +555,6 @@ def test_from_circuit_with_all_types(self):
expected_clifford = Clifford.from_dict(expected_clifford_dict)
self.assertEqual(combined_clifford, expected_clifford)

def test_from_gate_with_cyclic_definition(self):
"""Test if a Clifford can be created from gate with cyclic definition"""

class MyHGate(HGate):
"""Custom HGate class for test"""

def __init__(self):
super().__init__()
self.name = "my_h"

def _define(self):
qc = QuantumCircuit(1, name=self.name)
qc.s(0)
qc.append(MySXGate(), [0])
qc.s(0)
self.definition = qc

class MySXGate(SXGate):
"""Custom SXGate class for test"""

def __init__(self):
super().__init__()
self.name = "my_sx"

def _define(self):
qc = QuantumCircuit(1, name=self.name)
qc.sdg(0)
qc.append(MyHGate(), [0])
qc.sdg(0)
self.definition = qc

Clifford(MyHGate())


@ddt
class TestCliffordSynthesis(QiskitTestCase):
Expand Down

0 comments on commit 35f9e7c

Please sign in to comment.