Add `ObservablesArray.apply_layout()` #12221

ihincks · 2024-04-19T03:58:56Z

Summary

This PR adds the method ObservablesArray.apply_layout() which applies a transpiler layout to all elements of the array.

Details and comments

qiskit-bot · 2024-04-19T03:59:01Z

One or more of the the following people are requested to review this:

@Qiskit/terra-core
@ajavadia
@levbishop
@t-imamichi

coveralls · 2024-04-19T04:15:55Z

Pull Request Test Coverage Report for Build 9003200844

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

37 of 38 (97.37%) changed or added relevant lines in 1 file are covered.
125 unchanged lines in 16 files lost coverage.
Overall coverage increased (+0.02%) to 89.645%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/primitives/containers/observables_array.py	37	38	97.37%

Files with Coverage Reduction	New Missed Lines	%
qiskit/transpiler/passes/routing/star_prerouting.py	1	97.75%
qiskit/circuit/library/data_preparation/initializer.py	1	93.75%
qiskit/synthesis/clifford/clifford_decompose_ag.py	2	96.15%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py	2	88.02%
qiskit/circuit/library/standard_gates/xx_plus_yy.py	2	95.56%
qiskit/circuit/library/pauli_evolution.py	3	94.44%
qiskit/circuit/library/generalized_gates/linear_function.py	4	85.71%
crates/qasm2/src/lex.rs	5	92.62%
qiskit/circuit/library/generalized_gates/unitary.py	6	91.46%
qiskit/circuit/library/data_preparation/state_preparation.py	6	94.74%

Totals
Change from base Build 8930297650:	0.02%
Covered Lines:	62239
Relevant Lines:	69428

💛 - Coveralls

t-imamichi · 2024-04-19T10:29:51Z

It looks good.
We need to elaborate #11594 so that we can minimize the overhead of this apply_layout.
Padding "I" for all observables could be too heavy. Nice to have a sparse representation ignoring "I".

chriseclectic · 2024-04-24T17:04:32Z

This needs a bunch of unit tests, but other than that looks fine.

@t-imamichi agreed, though as long as we are happy with the function signature here we should add tests and merge, and then we can update its implementation to be more efficient later on when the internal data structure is improved for qubit sparsity.

t-imamichi · 2024-04-25T02:22:48Z

@chriseclectic Yes, I agree with you. We can improve the performance as a separate PR.

…y-layout

mtreinish

I think the code for this function looks correct; although I'm not super familiar with ObservablesArray data structure. However, ~~this is missing a release note~~ (never mind this was never public before so there isn't a need to document this as an addition to the class's interface) and also has an unrelated test file change which is causing test job failures.

test/python/transpiler/test_transpile_layout.py

This commit reverts accidental changes to the test module test_transpile_layout. These changes were causing the test to fail because the exepected layout was no longer complete.

test/python/primitives/containers/test_observables_array.py

ElePT

LGTM

t-imamichi · 2024-05-03T13:22:37Z

I have two questions.

The current implementation uses the pauli in the last qubit if there are some duplicate indices.
If it is intentional, it is nice to add a test. Otherwise, it might be good to raise an error if there are some duplicate indices.

a = ObservablesArray({"XYZ": 1})
print(a.apply_layout([1, 1, 1]))
# ObservablesArray({'IXI': 1.0}, shape=())

I don't get the following behavior. Could you check whether it is correct or not? I thought 5 is out of bounds.

a = ObservablesArray({"XYZ": 1})
print(a.apply_layout([0, 1, 5]))
# ObservablesArray({'XYZ': 1.0}, shape=())

mtreinish · 2024-05-03T13:35:34Z

I don't think [1, 1, 1] is a valid layout array, that should be an error on input. [0, 1, 5] would be valid only if you set num_qubits >= 6 but without that it should error. My concern at this point then is tagging the release though, it sounds like you think this isn't ready to merge yet. If so then we can defer this until 1.2.0.

t-imamichi · 2024-05-03T13:35:53Z

qiskit/primitives/containers/observables_array.py

+                new_key = n_qubits * ["I"]
+                for char, qubit in zip(reversed(key), layout):
+                    # Qubit position is from end of string
+                    new_key[n_qubits - 1 - qubit] = char


if n_qubits - 1 - qubit < 0, the second case of my comment #12221 (comment) might occur.

It might be nice to enforce n_qubits - 1 - qubit is non-negative.

mtreinish · 2024-05-03T13:44:23Z

Given the issues flagged by @t-imamichi and that we're already past the release target date I think it's safer if we just defer this to 1.2.0 at this point instead of trying to rush this in at the last minute, so I've retargeted this PR for the next release.

t-imamichi · 2024-05-03T14:20:59Z

I checked SparsePauliOp and found that the first case is same as SparsePauliOp and the second one raises an error.

In [11]: op = SparsePauliOp("XYZ")

In [12]: op
Out[12]:
SparsePauliOp(['XYZ'],
              coeffs=[1.+0.j])

In [13]: op.apply_layout([1, 1, 1])
Out[13]:
SparsePauliOp(['IXI'],
              coeffs=[0.-1.j])

In [14]: op.apply_layout([0, 1, 5])
---------------------------------------------------------------------------
QiskitError                               Traceback (most recent call last)
Cell In[14], line 1
----> 1 op.apply_layout([0, 1, 5])

File ~/tasks/4_2024/qiskit/terra/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py:1163, in SparsePauliOp.apply_layout(self, layout, num_qubits)
   1161     n_qubits = num_qubits
   1162 if layout is not None and any(x >= n_qubits for x in layout):
-> 1163     raise QiskitError("Provided layout contains indices outside the number of qubits.")
   1164 if layout is None:
   1165     layout = list(range(self.num_qubits))

QiskitError: 'Provided layout contains indices outside the number of qubits.'

chriseclectic · 2024-05-08T13:38:11Z

@t-imamichi For the current implementations of Pauli and SparsePauliOp, and this classes implementations, a list of ints for the layout is only able to apply a permutation of the current number of qubits unless you also specify num_qubits. This requirement is also stated in the apply_layout docstrings.

Eg these all work:

op1 = qi.Pauli("XYZ")
op2 = qi.SparsePauliOp(["XYZ"])
op3 = ObservablesArray(["XYZ"])
for op in [op1, op2, op3]:
    print(op.apply_layout([0, 1, 5], num_qubits=6))

If you remove the num_qubits kwarg, the qi ops error, while this current version does nothing, I've pushed a fix so that it will now raise the same errors as the qi ops if you don't do a permutation layout.

If we wanted to change this behavior to allow expansion via an integer layout for all apply_layout ops an easy fix would be to implicitly set num_qubits = 1 + max(layout) if num_qubits isn't provided and layout is a list of ints.

t-imamichi · 2024-05-08T13:56:18Z

Thank you for the information, Chris. I think it's enough to make the behavior of ObservableArray.apply_layout compatible with that of SparsePauliOp.apply_layout.

t-imamichi · 2024-05-09T02:53:06Z

(update) since #12385 was merged, the situation has changed. I will write a new comment below #12221 (comment). Please ignore this comment.

I'm checking the behavior of SparsePauliOp.apply_layout and ObservableArray.apply_layout.
I found some different cases as follows. I'm wondering how to make them compatible.

Case 1: negative indices

SparsePauliOp.apply_layout allows negative indices as follows while ObservableArray.apply_layout does not as follows.
Besides, the out-of-bound error for positive indices and negative indices are different. It raises QiskitError for positive indices and IndexError for negative indices. It would be update either one (IndexError might be more intuitive?)

op = SparsePauliOp("XYZ")
print(op.apply_layout([0, 1, -1])
# SparsePauliOp(['XYZ'],
#              coeffs=[1.+0.j])

print(op.apply_layout([0, 1, -2]))
# SparsePauliOp(['IXZ'],
#              coeffs=[0.-1.j])

print(op.apply_layout([0, 1, -3]))
# SparsePauliOp(['IYX'],
#              coeffs=[1.+0.j])

print(op.apply_layout([0, 1, -4]))
#    328 self._op_shape.compose(other._op_shape, qargs, front)
#    330 if qargs is not None:
#--> 331     x1, z1 = self.paulis.x[:, qargs], self.paulis.z[:, qargs]
#    332 else:
#    333     x1, z1 = self.paulis.x, self.paulis.z
#
#IndexError: index -4 is out of bounds for axis 1 with size 3

a = ObservablesArray({"XYZ": 1})
a.apply_layout([0, 1, -1])

#    170 new_key = n_qubits * ["I"]
#    171 for char, qubit in zip(reversed(key), layout):
#    172     # Qubit position is from end of string
#--> 173     new_key[n_qubits - 1 - qubit] = char
#    174 return "".join(new_key)
#
# IndexError: list assignment index out of range

Case 2: resulting coefficients for duplicate indices of layout

If we give duplicate indices, SparsePauliOp.apply_layout might return non-one coefficient while ObservableArray.apply_layout returns coefficient 1.
It does not make sense to me to change coefficients. Is it perhaps a bug of SparsePauliOp?

op = SparsePauliOp("XYZ")
print(op.apply_layout([0, 0, 0,])
# SparsePauliOp(['IIX'],
#               coeffs=[0.-1.j])

a = ObservablesArray({"XYZ": 1})
print(a.apply_layout([0, 0, 0])
# ObservablesArray({'IIX': 1.0}, shape=())

Case 3: layout whose size is smaller than the number of qubits

If we give a list whose size is smaller than the number of qubits as layout (e.g., []), SparsePauliOp.apply_layout and ObservableArray.apply_layout behave differently.

op = SparsePauliOp("XYZ")
print(op.apply_layout([]))
# File ~/tasks/4_2024/qiskit/terra/qiskit/quantum_info/operators/op_shape.py:477, in OpShape.compose(self, other, qargs, front)
#    475 ret._num_qargs_r = self._num_qargs_r
#    476 if len(qargs) != other._num_qargs_r:
#--> 477     raise QiskitError(
#    478         "Number of qargs does not match ({} != {})".format(
#    479             len(qargs), other._num_qargs_r
#    480         )
#    481     )
#    482 if self._dims_l or other._dims_l:
#    483     if self.dims_l(qargs) != other.dims_r():
#
# QiskitError: 'Number of qargs does not match (0 != 3)'

print(op.apply_layout([0], 1)
# File ~/tasks/4_2024/qiskit/terra/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py:1157, in SparsePauliOp.apply_layout(self, layout, num_qubits)
#   1155 if num_qubits is not None:
#   1156     if num_qubits < n_qubits:
#-> 1157         raise QiskitError(
#   1158             f"The input num_qubits is too small, a {num_qubits} qubit layout cannot be "
#   1159             f"applied to a {n_qubits} qubit operator"
#   1160         )
#   1161     n_qubits = num_qubits
#   1162 if layout is not None and any(x >= n_qubits for x in layout):
#
# QiskitError: 'The input num_qubits is too small, a 1 qubit layout cannot be applied to a 3 qubit operator'

a = ObservablesArray({"XYZ": 1})
print(a.apply_layout([]))
# ObservablesArray({'': 1.0}, shape=())

print(a.apply_layout([0], 1))
# ObservablesArray({'Z': 1.0}, shape=())

t-imamichi · 2024-05-09T03:19:35Z

I found a corner case of SparsePauliOp.apply_layout and made a PR #12375. ObservableArray.apply_layout works fine with the case. But it might be nice to port the tests.

t-imamichi · 2024-05-10T10:20:19Z

I made a PR #12385 so that SparsePauliOp and Pauli raise an error for duplicate indices or negative indices.

t-imamichi · 2024-05-10T13:57:10Z

There are some different behaviors between SparsePauliOp.apply_layout and ObservableArray.apply_layout as follows.

Case 1: negative indices

SparsePauliOp.apply_layout raises QiskitError with negative indices while ObservableArray.apply_layout raises IndexError. Since they both raise QiskitError with out-of-bounds indices. It's good to update ObservableArray.apply_layout to raise QiskitError with negative indices.

op = SparsePauliOp("XYZ")
print(op.apply_layout([0, 1, -1])
# QiskitError: 'Provided layout contains indices outside the number of qubits.'

a = ObservablesArray({"XYZ": 1})
a.apply_layout([0, 1, -1])

#    170 new_key = n_qubits * ["I"]
#    171 for char, qubit in zip(reversed(key), layout):
#    172     # Qubit position is from end of string
#--> 173     new_key[n_qubits - 1 - qubit] = char
#    174 return "".join(new_key)
#
# IndexError: list assignment index out of range

Case 2: duplicate indices

SparsePauliOp.apply_layout raises QiskitError with duplicate indices while ObservableArray.apply_layout allows it. I agree with Matthew that duplicate indices are not valid as a layout #12221 (comment).

op = SparsePauliOp("XYZ")
print(op.apply_layout([0, 0, 0])
# QiskitError: 'Provided layout contains duplicate indices.'

a = ObservablesArray({"XYZ": 1})
print(a.apply_layout([0, 0, 0])
# ObservablesArray({'IIX': 1.0}, shape=())

Case 3: layout whose size is smaller than the number of qubits

If we give a list whose size is smaller than the number of qubits as layout (e.g., []), SparsePauliOp.apply_layout and ObservableArray.apply_layout behave differently.
I'm wondering which behavior is more intuitive.

op = SparsePauliOp("XYZ")
print(op.apply_layout([]))
# File ~/tasks/4_2024/qiskit/terra/qiskit/quantum_info/operators/op_shape.py:477, in OpShape.compose(self, other, qargs, front)
#    475 ret._num_qargs_r = self._num_qargs_r
#    476 if len(qargs) != other._num_qargs_r:
#--> 477     raise QiskitError(
#    478         "Number of qargs does not match ({} != {})".format(
#    479             len(qargs), other._num_qargs_r
#    480         )
#    481     )
#    482 if self._dims_l or other._dims_l:
#    483     if self.dims_l(qargs) != other.dims_r():
#
# QiskitError: 'Number of qargs does not match (0 != 3)'

print(op.apply_layout([0], 1)
# File ~/tasks/4_2024/qiskit/terra/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py:1157, in SparsePauliOp.apply_layout(self, layout, num_qubits)
#   1155 if num_qubits is not None:
#   1156     if num_qubits < n_qubits:
#-> 1157         raise QiskitError(
#   1158             f"The input num_qubits is too small, a {num_qubits} qubit layout cannot be "
#   1159             f"applied to a {n_qubits} qubit operator"
#   1160         )
#   1161     n_qubits = num_qubits
#   1162 if layout is not None and any(x >= n_qubits for x in layout):
#
# QiskitError: 'The input num_qubits is too small, a 1 qubit layout cannot be applied to a 3 qubit operator'

a = ObservablesArray({"XYZ": 1})
print(a.apply_layout([]))
# ObservablesArray({'': 1.0}, shape=())

print(a.apply_layout([0], 1))
# ObservablesArray({'Z': 1.0}, shape=())

Add ObservablesArray.apply_layout()

5b25adf

ihincks added the mod: primitives Related to the Primitives module label Apr 19, 2024

ihincks added this to the 1.1.0 milestone Apr 19, 2024

ihincks requested review from a team as code owners April 19, 2024 03:58

ihincks added 3 commits May 2, 2024 15:38

Merge remote-tracking branch 'upstream/main' into primitives/obs-arra…

f35c663

…y-layout

add apply_layout() tests

5581675

fix linting

f8623db

mtreinish reviewed May 3, 2024

View reviewed changes

test/python/transpiler/test_transpile_layout.py Outdated Show resolved Hide resolved

mtreinish added 2 commits May 3, 2024 06:12

Revert test changes in test_transpile_layout

b93e0cc

This commit reverts accidental changes to the test module test_transpile_layout. These changes were causing the test to fail because the exepected layout was no longer complete.

Merge branch 'main' into primitives/obs-array-layout

3219bd7

mtreinish reviewed May 3, 2024

View reviewed changes

test/python/primitives/containers/test_observables_array.py Outdated Show resolved Hide resolved

Fix lint

e6559cc

mtreinish added the Changelog: None Do not include in changelog label May 3, 2024

ElePT previously approved these changes May 3, 2024

View reviewed changes

ElePT enabled auto-merge May 3, 2024 11:00

Appease pylint's arbitrary import order rules

1dff034

mtreinish dismissed ElePT’s stale review via 1dff034 May 3, 2024 12:26

ElePT previously approved these changes May 3, 2024

View reviewed changes

ElePT disabled auto-merge May 3, 2024 13:29

t-imamichi reviewed May 3, 2024

View reviewed changes

mtreinish modified the milestones: 1.1.0, 1.2.0 May 3, 2024

garrison mentioned this pull request May 3, 2024

Create how-to optimize transpilation guide Qiskit-Extensions/circuit-knitting-toolbox#573

Draft

4 tasks

Fix exception raising when integer layout is not a permutation

94bf141

chriseclectic dismissed ElePT’s stale review via 94bf141 May 8, 2024 13:54

Fix test for error type

807333b

t-imamichi mentioned this pull request May 10, 2024

Fix {Pauli,SparsePauliOp}.apply_layout to raise an error with negative or duplicate indices #12385

Merged

mergify bot mentioned this pull request May 10, 2024

Fix {Pauli,SparsePauliOp}.apply_layout to raise an error with negative or duplicate indices (backport #12385) #12386

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `ObservablesArray.apply_layout()` #12221

Add `ObservablesArray.apply_layout()` #12221

ihincks commented Apr 19, 2024

qiskit-bot commented Apr 19, 2024

coveralls commented Apr 19, 2024 •

edited

t-imamichi commented Apr 19, 2024

chriseclectic commented Apr 24, 2024 •

edited

t-imamichi commented Apr 25, 2024

mtreinish left a comment •

edited

ElePT left a comment

t-imamichi commented May 3, 2024 •

edited

mtreinish commented May 3, 2024

t-imamichi May 3, 2024

mtreinish commented May 3, 2024 •

edited

t-imamichi commented May 3, 2024

chriseclectic commented May 8, 2024 •

edited

t-imamichi commented May 8, 2024

t-imamichi commented May 9, 2024 •

edited

t-imamichi commented May 9, 2024

t-imamichi commented May 10, 2024

t-imamichi commented May 10, 2024 •

edited

Add ObservablesArray.apply_layout() #12221

Are you sure you want to change the base?

Add ObservablesArray.apply_layout() #12221

Conversation

ihincks commented Apr 19, 2024

Summary

Details and comments

qiskit-bot commented Apr 19, 2024

coveralls commented Apr 19, 2024 • edited

Pull Request Test Coverage Report for Build 9003200844

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

t-imamichi commented Apr 19, 2024

chriseclectic commented Apr 24, 2024 • edited

t-imamichi commented Apr 25, 2024

mtreinish left a comment • edited

Choose a reason for hiding this comment

ElePT left a comment

Choose a reason for hiding this comment

t-imamichi commented May 3, 2024 • edited

mtreinish commented May 3, 2024

t-imamichi May 3, 2024

Choose a reason for hiding this comment

mtreinish commented May 3, 2024 • edited

t-imamichi commented May 3, 2024

chriseclectic commented May 8, 2024 • edited

t-imamichi commented May 8, 2024

t-imamichi commented May 9, 2024 • edited

Case 1: negative indices

Case 2: resulting coefficients for duplicate indices of layout

Case 3: layout whose size is smaller than the number of qubits

t-imamichi commented May 9, 2024

t-imamichi commented May 10, 2024

t-imamichi commented May 10, 2024 • edited

Case 1: negative indices

Case 2: duplicate indices

Case 3: layout whose size is smaller than the number of qubits

Add `ObservablesArray.apply_layout()` #12221

Add `ObservablesArray.apply_layout()` #12221

coveralls commented Apr 19, 2024 •

edited

chriseclectic commented Apr 24, 2024 •

edited

mtreinish left a comment •

edited

t-imamichi commented May 3, 2024 •

edited

mtreinish commented May 3, 2024 •

edited

chriseclectic commented May 8, 2024 •

edited

t-imamichi commented May 9, 2024 •

edited

t-imamichi commented May 10, 2024 •

edited