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

Add label assigned check in SparsePauliOp #8101

Merged
merged 6 commits into from
Jun 14, 2022
Merged

Add label assigned check in SparsePauliOp #8101

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f2b09e1
add assign check
daimurat May 23, 2022
0f37f94
Update qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py
daimurat May 24, 2022
4d9db73
fix check condition
daimurat May 27, 2022
74c7c39
update reno
Cryoris Jun 14, 2022
82af8ed
update reno section header
Cryoris Jun 14, 2022
4ddb629
Merge branch 'main' into issue_#7916
mergify[bot] Jun 14, 2022
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
8 changes: 7 additions & 1 deletion qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ def from_list(obj):
return SparsePauliOp(paulis, coeffs, copy=False)

@staticmethod
def from_sparse_list(obj, num_qubits):
def from_sparse_list(obj, num_qubits, do_checks=True):
"""Construct from a list of local Pauli strings and coefficients.

Each list element is a 3-tuple of a local Pauli string, indices where to apply it,
Expand All @@ -636,13 +636,15 @@ def from_sparse_list(obj, num_qubits):
Args:
obj (Iterable[Tuple[str, List[int], complex]]): The list 3-tuples specifying the Paulis.
num_qubits (int): The number of qubits of the operator.
do_checks (bool): The flag of checking if it is already assigned (not I).
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 I should be checked too, otherwise it's unclear how we interpret using an index multiple times 🙂 Could you add that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you @Cryoris.
How about not to being allowed input I instead of checking I ?
I think there seems to be no use case that input I should be applied.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is true that there is no reason to pass I with sparse format, so I don't think we need to allow I.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that's true, strictly speaking there's not reason to pass I. But it might be restrictive to explicitly forbid it because (1) if a user happens to have strings with I in them they would have to clean the string, which can be cumbersome (maybe the string is automatically constructed) and (2) we would have to add a check whether I is included. I think it would be easier for both users and us if we just allow any Pauli string.

We could of course clean the input strings and only store the non-I ones, that might be good though!

Copy link
Contributor

Choose a reason for hiding this comment

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

Then I feel that it is ok to not check I for the first version. Personally, I am concerned that it would complicate the implementation. As long as we make errors as long as the output is clearly strange (as issue pointed out), we don't have problems.

Copy link
Contributor

@Cryoris Cryoris May 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

There are a few ways to implement it which a not complicating the implementation I think 🙂 For example one of the following 2:

  1. Just check if indices contains duplicated (that might be even simpler than the current code 🙂)
if do_checks and np.unique(indices).size == len(indices):
  # raise error
  1. At the beginning, populate labels with None and use the check that you currently have. Then at the end replace all left Nones with "I" 🙂

Copy link
Contributor

@ikkoham ikkoham May 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Both has extra computational cost, but I'd prefer 1 using set.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you. I fixed the check condition. Could you review it?


Returns:
SparsePauliOp: The SparsePauliOp representation of the Pauli terms.

Raises:
QiskitError: If the list of Paulis is empty.
QiskitError: If the number of qubits is incompatible with the indices of the Pauli terms.
QiskitError: If the designated qubit is already assigned.
"""
obj = list(obj) # To convert zip or other iterable

Expand All @@ -661,6 +663,10 @@ def from_sparse_list(obj, num_qubits):
raise QiskitError(
f"The number of qubits ({num_qubits}) is smaller than a required index {index}."
)
if do_checks is True and label[~index] in ("X", "Y", "Z"):
daimurat marked this conversation as resolved.
Show resolved Hide resolved
raise QiskitError(
f"The qubit (index: {num_qubits}) is already assigned {label}."
)
label[~index] = pauli

labels[i] = "".join(label)
Expand Down
6 changes: 6 additions & 0 deletions releasenotes/notes/add_check_from_sparse_list-97f13fde87c7bcb6.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
upgrade:
Cryoris marked this conversation as resolved.
Show resolved Hide resolved
- |
Added a check process and a flag which is used to check if the qubit designated from input Pauli list is already assigned (not I)
to :func:`from_sparse_list` in the :class:`SparsePauliOp`. The flag is used if check is to be run. The default value is True.
See `#7916 <https://github.com/Qiskit/qiskit-terra/issues/7916>`__.
7 changes: 7 additions & 0 deletions test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,13 @@ def test_from_index_list_raises(self):
with self.assertRaises(QiskitError):
_ = SparsePauliOp.from_sparse_list([("Z", [2], 1)], 1)

def test_from_index_list_same_index(self):
"""Test from_list via Pauli + number of qubits raises correctly, if indices duplicate."""
with self.assertRaises(QiskitError):
_ = SparsePauliOp.from_sparse_list([("ZZ", [0, 0], 1)], 2)
with self.assertRaises(QiskitError):
_ = SparsePauliOp.from_sparse_list([("ZI", [0, 0], 1)], 2)

def test_from_zip(self):
"""Test from_list method for zipped input."""
labels = ["XXZ", "IXI", "YZZ", "III"]
Expand Down