-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add .apply_layout()
method to Pauli
class
#12066
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 8644607674Warning: 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.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, just a whitespace nit inline and a small question. But otherwise this is straightforward as it's just the same method as SparsePauliOp
.
raise QiskitError("Provided layout contains indices outside the number of qubits.") | ||
if layout is None: | ||
layout = list(range(self.num_qubits)) | ||
new_op = type(self)("I" * n_qubits) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize I did the same thing in #10947 but I'm wondering if there is a more efficient constructor here that doesn't require creating a giant string like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, as far as I know, there is no good way to "sparsely" define long operators with only a few non-identity terms, in fact, I think that this is exactly what this issue describes (the word "layout" has a different meaning here: #11891).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, you can directly generate the symplectic Boolean matrices, but fundamentally, Pauli
and SparsePauliOp
aren't sparse over qubit number - the latter is "sparse" over the full Pauli vector.
Max's issue is about that sort of thing, yeah, although practically I don't think it's something we'll be able to put directly into Pauli
and SparsePauliOp
- I think it's something we'd do as new QI classes, probably directly in Rust space. This it the kind of thing that Paul's been interested in before as well, and similar to how OpenFermion and friends represent operators on huge spaces. The primitives want that as well.
Co-authored-by: Matthew Treinish <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests look right to me (and definitely something we should have), thanks for doing it!
def test_apply_layout_with_transpile(self): | ||
"""Test the apply_layout method with a transpiler layout.""" | ||
psi = EfficientSU2(4, reps=4, entanglement="circular") | ||
op = Pauli("IZZZ") | ||
backend = GenericBackendV2(num_qubits=7) | ||
transpiled_psi = transpile(psi, backend, optimization_level=3, seed_transpiler=12345) | ||
permuted_op = op.apply_layout(transpiled_psi.layout) | ||
identity_op = Pauli("I" * 7) | ||
initial_layout = transpiled_psi.layout.initial_index_layout(filter_ancillas=True) | ||
final_layout = transpiled_psi.layout.routing_permutation() | ||
qargs = [final_layout[x] for x in initial_layout] | ||
expected_op = identity_op.compose(op, qargs=qargs) | ||
self.assertNotEqual(op, permuted_op) | ||
self.assertEqual(permuted_op, expected_op) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for completeness' sake, could you add a test that SparsePauliOp(Pauli(p).apply_layout(layout))
produces the same result as SparsePauliop(Pauli(p)).apply_layout(layout)
? Just a consistency test, to enforce that we don't forget one or the other while updating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that you were meaning to say SparsePauliOp(Pauli(p).apply_layout(layout))
produces the same result as Pauli(p).apply_layout(layout)
, right? This is what I added in 434a21c.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's fine too - you've ended up reducing the SparsePauliOp
to a PauliList
to check, whereas I was suggesting raising the Pauli
to a SparsePauliOp
to compare, but both test the same thing that I wanted to test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahaa, I didn't notice the brackets. I can also do it the way you originally suggested (looks cleaner to compare the exact same type).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the extra test!
Summary
This PR adds an
apply_layout
method to thePauli
class, following what was done forSparsePauliOp
in #10947. This was the agreed action to partially solve #11824 in the short term until a more permanent/extensible solution is designed.Details and comments