Skip to content

Fix zero-qubit Pauli label strings #9726

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

Merged
merged 3 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 8 additions & 37 deletions qiskit/quantum_info/operators/symplectic/pauli.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ class initialization (``Pauli('-iXYZ')``). A ``Pauli`` object can be
# Set the max Pauli string size before truncation
__truncate__ = 50

_VALID_LABEL_PATTERN = re.compile(r"^[+-]?1?[ij]?[IXYZ]+$")
_VALID_LABEL_PATTERN = re.compile(r"(?P<coeff>[+-]?1?[ij]?)(?P<pauli>[IXYZ]*)")
_CANONICAL_PHASE_LABEL = {"": 0, "-i": 1, "-": 2, "i": 3}

def __init__(self, data=None, x=None, *, z=None, label=None):
"""Initialize the Pauli.
Expand Down Expand Up @@ -613,17 +614,15 @@ def _from_label(label):
Raises:
QiskitError: if Pauli string is not valid.
"""
if Pauli._VALID_LABEL_PATTERN.match(label) is None:
match_ = Pauli._VALID_LABEL_PATTERN.fullmatch(label)
if match_ is None:
raise QiskitError(f'Pauli string label "{label}" is not valid.')

# Split string into coefficient and Pauli
pauli, coeff = _split_pauli_label(label)

# Convert coefficient to phase
phase = 0 if not coeff else _phase_from_label(coeff)
phase = Pauli._CANONICAL_PHASE_LABEL[
(match_["coeff"] or "").replace("1", "").replace("+", "").replace("j", "i")
]

# Convert to Symplectic representation
pauli_bytes = np.frombuffer(pauli.encode("ascii"), dtype=np.uint8)[::-1]
pauli_bytes = np.frombuffer(match_["pauli"].encode("ascii"), dtype=np.uint8)[::-1]
ys = pauli_bytes == ord("Y")
base_x = np.logical_or(pauli_bytes == ord("X"), ys).reshape(1, -1)
base_z = np.logical_or(pauli_bytes == ord("Z"), ys).reshape(1, -1)
Expand Down Expand Up @@ -698,33 +697,5 @@ def _from_circuit(cls, instr):
return ret._z, ret._x, ret._phase


# ---------------------------------------------------------------------
# Label parsing helper functions
# ---------------------------------------------------------------------


def _split_pauli_label(label):
"""Split Pauli label into unsigned group label and coefficient label"""
span = re.search(r"[IXYZ]+", label).span()
pauli = label[span[0] :]
coeff = label[: span[0]]
if span[1] != len(label):
invalid = set(re.sub(r"[IXYZ]+", "", label[span[0] :]))
raise QiskitError(
f"Pauli string contains invalid characters {invalid} ∉ ['I', 'X', 'Y', 'Z']"
)
return pauli, coeff


def _phase_from_label(label):
"""Return the phase from a label"""
# Returns None if label is invalid
label = label.replace("+", "", 1).replace("1", "", 1).replace("j", "i", 1)
phases = {"": 0, "-i": 1, "-": 2, "i": 3}
if label not in phases:
raise QiskitError(f"Invalid Pauli phase label '{label}'")
return phases[label]


# Update docstrings for API docs
generate_apidocs(Pauli)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
fixes:
- |
Fixed an edge case in the construction of :class:`.Pauli` instances; a string with an optional
phase and no qubits is now a valid label, making an operator with no qubits (such as
``Pauli("-i")``). This was already possible when using the array forms, or empty slices.
Fixed `#9720 <https://github.com/Qiskit/qiskit-terra/issues/9720>`__.
5 changes: 1 addition & 4 deletions test/python/opflow/test_op_construction.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
Zero,
)
from qiskit.quantum_info import Operator, Pauli, Statevector
from qiskit.quantum_info.operators.symplectic.pauli import _phase_from_label, _split_pauli_label

# pylint: disable=invalid-name

Expand Down Expand Up @@ -1242,9 +1241,7 @@ def pauli_group_labels(nq, full_group=True):

def operator_from_label(label):
"""Construct operator from full Pauli group label"""
pauli, coeff = _split_pauli_label(label)
coeff = (-1j) ** _phase_from_label(coeff)
return coeff * Operator.from_label(pauli)
return Operator(Pauli(label))


@ddt
Expand Down
2 changes: 1 addition & 1 deletion test/python/primitives/test_estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ def test_validate_observables(self, obsevables, expected):
"""Test obsevables standardization."""
self.assertEqual(BaseEstimator._validate_observables(obsevables), expected)

@data(None, "ERROR", "")
@data(None, "ERROR")
def test_qiskit_error(self, observables):
"""Test qiskit error if invalid input."""
with self.assertRaises(QiskitError):
Comment on lines -699 to 702
Copy link
Member Author

Choose a reason for hiding this comment

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

This got removed because "" is a valid input now - it's not that the estimator fails to reject it, it's that it shouldn't be rejected now. It'll very likely get rejected when somebody tries to actually run a circuit with a Pauli("") as the observable, because I'm assuming that nobody's running zero-qubit circuits and so the number of qubits won't match.

Copy link
Member

Choose a reason for hiding this comment

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

and so the number of qubits won't match

Actually, if #9700 is merged in its current form, a zero-qubit observable would be allowed for any circuit (i.e., with any number of qubits) as far as BaseEstimator is concerned, if I understand correctly.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's pretty wild to me. If you don't specify which qubits you're acting on with the observable, how is the primitive meant to know which ones undergo the implicit identity - is it the first qubit indices that it applies to? That doesn't seem right. Anyway, I don't know the context of that PR, and it wouldn't be my call anyway.

I think this PR is correct regardless of that, though.

Copy link
Member

Choose a reason for hiding this comment

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

I think this PR is correct regardless of that, though.

I agree.

Expand Down
24 changes: 23 additions & 1 deletion test/python/quantum_info/operators/symplectic/test_pauli.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

"""Tests for Pauli operator class."""

import re
import unittest
import itertools as it
from functools import lru_cache
Expand Down Expand Up @@ -41,7 +42,19 @@

from qiskit.quantum_info.random import random_clifford, random_pauli
from qiskit.quantum_info.operators import Pauli, Operator
from qiskit.quantum_info.operators.symplectic.pauli import _split_pauli_label, _phase_from_label

LABEL_REGEX = re.compile(r"(?P<coeff>[+-]?1?[ij]?)(?P<pauli>[IXYZ]*)")
PHASE_MAP = {"": 0, "-i": 1, "-": 2, "i": 3}


def _split_pauli_label(label):
match_ = LABEL_REGEX.fullmatch(label)
return match_["pauli"], match_["coeff"]


def _phase_from_label(label):
coeff = LABEL_REGEX.fullmatch(label)["coeff"] or ""
return PHASE_MAP[coeff.replace("+", "").replace("1", "").replace("j", "i")]


@lru_cache(maxsize=8)
Expand Down Expand Up @@ -462,6 +475,15 @@ def test_barrier_delay_sim(self):
value = Pauli(circ)
self.assertEqual(value, target)

@data(("", 0), ("-", 2), ("i", 3), ("-1j", 1))
@unpack
def test_zero_qubit_pauli_construction(self, label, phase):
"""Test that Paulis of zero qubits can be constructed."""
expected = Pauli(label + "X")[0:0] # Empty slice from a 1q Pauli, which becomes phaseless
expected.phase = phase
test = Pauli(label)
self.assertEqual(expected, test)


if __name__ == "__main__":
unittest.main()