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.

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

pauli string reps added (#6243) #6562

Merged
merged 60 commits into from
Dec 5, 2024
Merged

pauli string reps added (#6243) #6562

merged 60 commits into from
Dec 5, 2024

Conversation

ldi18
Copy link
Contributor

@ldi18 ldi18 commented Nov 10, 2024

Context:
Prior to this commit, PennyLane only allowed access to Pauli string representations for the X, Z and Z gate via X(0).pauli_rep, etc. The Pauli representations of other gates were not implemented.

Description of the Change:
I added the Pauli string representations for X, Y, Z, S, T, SX, SWAP, ISWAP, ECR, SISWAP.

The correctness of each Pauli string representation is checked by converting the returned Pauli string into a matrix and comparing it with the respective .matrix() method.

(Pauli reps for parametrized ops and H were not added as they caused problems in the CI tests.)

Benefits:

Pennylane now supports Pauli string decompositions for various operators. Pauli string decompositions of products of operators with available Pauli string decompositions can now be directly accessed. Example: (Hadamard(0) @ S(1) @ T(1, 2) @ SX(1) @ SWAP([0, 1])).pauli_rep

Related GitHub Issues:
#6243

Related Shortcut Stories:
[sc-73280]

@albi3ro albi3ro requested a review from dwierichs November 11, 2024 16:40
Copy link
Contributor

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

Thanks for this nice addition @ldi18 🎉
I had a couple of comments and proposed changes.

Nice catch regarding the PCPhase bug! If it is possible, it would be great to add a dedicated test for this bug. We know that it has been caught by the current test suite, but only in a very indirect manner. So it would be nice to check correctness of PauliSentence.to_mat or something like that directly in a small test.

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/ops/qubit/non_parametric_ops.py Outdated Show resolved Hide resolved
pennylane/ops/qubit/non_parametric_ops.py Outdated Show resolved Hide resolved
tests/ops/qubit/test_non_parametric_ops.py Outdated Show resolved Hide resolved
tests/ops/qubit/test_non_parametric_ops.py Outdated Show resolved Hide resolved
tests/ops/qubit/test_parametric_ops.py Outdated Show resolved Hide resolved
tests/ops/qubit/test_parametric_ops.py Outdated Show resolved Hide resolved
tests/ops/qubit/test_parametric_ops.py Outdated Show resolved Hide resolved
@ldi18
Copy link
Contributor Author

ldi18 commented Nov 13, 2024

@dwierichs Thanks for the feedback! Regarding the tensor-like parameter bug: I could implement shape-based test cases like

assert qml.RX([0.1, 0.23, 1.11], wires=0).matrix().shape == (3, 2, 2)

Such test cases should then also include products of Pauli sentences like

assert (qml.RX([1, 2, 3], wires=0) @ qml.RY([1, 2, 3], wires=0)).matrix().shape == (3, 2, 2)

The test case above actually fails unless I use

(qml.RX(np.array([1, 2, 3]), wires=0) @ qml.RY(np.array([1, 2, 3]), wires=0)).matrix().shape == (3, 2, 2)

It seems there might be a missing list-to-array conversion. Are lists not considered tensor-like in this context? Should I open a new issue for this and include more test cases for Pauli arithmetic in a PR linked to that issue?

@dwierichs
Copy link
Contributor

Hi @ldi18
I'm actually unsure how much further we want dive into the whole batched-parameter + Pauli arithmetic topic, because it opens up a whole can of worms regarding batching support in PauliSentence and the way qml.matrix computes matrices.
Do we have any way to circumvent this problem in this PR without fixing the underlying bug in this PR?
We could, for example, skip adding the pauli rep to PhaseShift.
It's not a beautiful solution, but it allows us to disentangle the batching support from this PR.

cc @albi3ro @isaacdevlugt

@ldi18
Copy link
Contributor Author

ldi18 commented Nov 13, 2024

Do we have any way to circumvent this problem in this PR without fixing the underlying bug in this PR? We could, for example, skip adding the pauli rep to PhaseShift. It's not a beautiful solution, but it allows us to disentangle the batching support from this PR.

@dwierichs Not sure if I get you right but the batching problem in pauli arithmetic applied to all parametrized gates with a pauli_rep. This can be seen from the test cases I added now, since they all fail when I revert the changes in _sum_same_structure_pws_dense().

    def test_tensor_like_inputs(self):
        """Tests that the shape of (flattened) tensor like inputs is preserved in pauli arithmetic."""
        assert qml.RX([0.1, 0.23, 1.11], wires=0).matrix().shape == (3, 2, 2)
        assert (
            qml.U1([[0.1, 0.23, 1.11, 2.34], [1.1, 1.23, 2.11, 3.34], [3.1, 3.23, 3.11, 3.34], [1.1, 1.23, 1.11, 1.34]],
                wires="qubit0",
            ).matrix().shape)
        assert (qml.RX([1.11, 2.11, 3.11], wires=0) @ qml.RY([1.23, 2.23, 3.23], wires=0)).matrix().shape == (3, 2, 2)
        assert (qml.RX([1.11, 2.11, 3.11], wires=0) @ qml.RY([1.23, 2.23, 3.23], wires="qubit1")).matrix().shape == (3, 4, 4)
        assert (qml.RX([1.11, 2.11, 3.11], wires="qubit0") @ qml.RY([1.23, 2.23, 3.23], wires="qubit1") @ qml.RY([1.23, 2.23, 3.23], wires="qubit3")).matrix().shape == (3, 8, 8)
        assert (qml.IsingXX([1.11, 2.11, 3.11, 4.11, 5.11], wires=[1, 2]) @ qml.PhaseShift([1.23, 2.23, 3.23, 4.23, 5.23], wires=2) @ qml.PCPhase([1.23, 2.23, 3.23, 4.23, 4.23], dim=2, wires=1)
        ).matrix().shape == (5, 4, 4)
        assert (qml.Rot([1.11, 2.11, 3.11, 4.11, 5.11], [1.11, 2.11, 3.11, 4.11, 5.11], [1.11, 2.11, 3.11, 4.11, 5.11], wires=0) @ qml.Rot([1.11, 2.11, 3.11, 4.11, 5.11], [1.11, 2.11, 3.11, 4.11, 5.11], [1.11, 2.11, 3.11, 4.11, 5.11], wires=1)).matrix().shape == (5, 4, 4)

Actually I can't reproduce the error mentioned above from

assert (qml.RX([1, 2, 3], wires=0) @ qml.RY([1, 2, 3], wires=0)).matrix().shape == (3, 2, 2)

so it was probably just a problem on my setup. Tensor-like parameters seem to work well as long as they are flattened. For example

print(qml.RX([[0.1, 0.23, 1.11, 2.34], [1.1, 1.23, 2.11, 3.34], [3.1, 3.23, 3.11, 3.34], [1.1, 1.23, 1.11, 1.34]], wires="qubit0").matrix().shape)
print(qml.U1([[0.1, 0.23, 1.11, 2.34], [1.1, 1.23, 2.11, 3.34], [3.1, 3.23, 3.11, 3.34], [1.1, 1.23, 1.11, 1.34]], wires="qubit0").matrix().shape)

gives me different shapes - but that goes beyond this PR.

@dwierichs
Copy link
Contributor

Ah, I think we indeed usually do not allow lists for batched inputs. Sorry I had missed to answer this part.

I agree that the bug affects all gates, but without adding your test cases, skipping the pauli_rep of PhaseShift will allow us to proceed with this PR and do the batching-related things in a new PR ;-)

@ldi18
Copy link
Contributor Author

ldi18 commented Dec 1, 2024

Hi @dwierichs,
Thanks for the help. I deleted all the pauli reps for the parametrized ops and the Hadamard gate from this PR. I also removed the corresponding tests and added a few pylint exceptions / made minor changes in test_non_parametric_ops required by pylint.

Copy link

codecov bot commented Dec 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.64%. Comparing base (a47e4c5) to head (f36b34d).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6562   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files         467      467           
  Lines       44119    44166   +47     
=======================================
+ Hits        43963    44010   +47     
  Misses        156      156           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ldi18 ldi18 requested a review from dwierichs December 1, 2024 22:54
Copy link
Contributor

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

Thanks for the update to the new version @ldi18 Looks good to me! 💯

For some reason, this PR is cursed and now has a technical problem in a Github action after testing the documentation creation. I'll follow up internally on this.

@Alex-Preciado Alex-Preciado removed the request for review from JerryChen97 December 4, 2024 18:02
@andrijapau andrijapau self-requested a review December 4, 2024 18:59
Copy link
Contributor

@andrijapau andrijapau left a comment

Choose a reason for hiding this comment

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

Minor code suggestion to make pylint happy, but otherwise it LGTM! Thanks for all your help! 🏅

.gitignore Outdated Show resolved Hide resolved
@dwierichs
Copy link
Contributor

Just waiting for #6661 to be merged, and then this PR should be merge-ready :)

@dwierichs dwierichs added the merge-ready ✔️ All tests pass and the PR is ready to be merged. label Dec 4, 2024
@dwierichs dwierichs enabled auto-merge (squash) December 5, 2024 15:25
@dwierichs dwierichs merged commit ce9e5ff into PennyLaneAI:master Dec 5, 2024
46 checks passed
dwierichs added a commit that referenced this pull request Dec 11, 2024
…ition to PR: #6562) (#6587)

**Context:**
I added Pauli string decompositions for single qubit parametrized gates
in PR: #6562. Pennylane exploits these Pauli string representations
when, for example, computing

```python
(qml.RX(1.11, wires=0) @ qml.RY(1.23, wires=0)).matrix()
```
The example above works as intended, but

```python
(qml.RX([1.11, 2.11, 3.11], wires=0) @ qml.RY([1.23, 2.23, 3.23], wires=0)).matrix()     # shape error
```
or
```python
(qml.RX(np.array([1.11, 2.11, 3.11]), wires=0) @ qml.RY(np.array([1.23, 2.23, 3.23]), wires=0)).matrix()     # shape error
```
returns a shape error. The code above should work since `RX`and
`RY`support tensor-like (flattened) parameters as it can be seen from
```python
qml.RX([1.11, 2.11, 3.11], wires=0).matrix()    # works!
```


**Description of the Change:**

I added an outer product in `pauli_arithmetic`.

**Benefits:**
- Pauli representations can now be leveraged for the matrix conversion
of products of ops.
- A Pauli representation of the `PhaseShift gate` could be added - this
was not possible before as the `PCPhase` gate used the
`pauli_arithmetic`, which gave an error in the pytests.

**Possible Drawbacks:**
None

**Related GitHub Issues:**
#6243 and PR: #6562

---------

Co-authored-by: Lasse Dierich <[email protected]>
Co-authored-by: David Wierichs <[email protected]>
Co-authored-by: Christina Lee <[email protected]>
mudit2812 pushed a commit that referenced this pull request Dec 13, 2024
…ition to PR: #6562) (#6587)

**Context:**
I added Pauli string decompositions for single qubit parametrized gates
in PR: #6562. Pennylane exploits these Pauli string representations
when, for example, computing

```python
(qml.RX(1.11, wires=0) @ qml.RY(1.23, wires=0)).matrix()
```
The example above works as intended, but

```python
(qml.RX([1.11, 2.11, 3.11], wires=0) @ qml.RY([1.23, 2.23, 3.23], wires=0)).matrix()     # shape error
```
or
```python
(qml.RX(np.array([1.11, 2.11, 3.11]), wires=0) @ qml.RY(np.array([1.23, 2.23, 3.23]), wires=0)).matrix()     # shape error
```
returns a shape error. The code above should work since `RX`and
`RY`support tensor-like (flattened) parameters as it can be seen from
```python
qml.RX([1.11, 2.11, 3.11], wires=0).matrix()    # works!
```


**Description of the Change:**

I added an outer product in `pauli_arithmetic`.

**Benefits:**
- Pauli representations can now be leveraged for the matrix conversion
of products of ops.
- A Pauli representation of the `PhaseShift gate` could be added - this
was not possible before as the `PCPhase` gate used the
`pauli_arithmetic`, which gave an error in the pytests.

**Possible Drawbacks:**
None

**Related GitHub Issues:**
#6243 and PR: #6562

---------

Co-authored-by: Lasse Dierich <[email protected]>
Co-authored-by: David Wierichs <[email protected]>
Co-authored-by: Christina Lee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-ready ✔️ All tests pass and the PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants