-
Notifications
You must be signed in to change notification settings - Fork 30
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
Adding tests for all PennyLane transforms #1215
base: main
Are you sure you want to change the base?
Conversation
…l add support to qml.qml.CosineWindow when it's called as the first op in tapes
Co-authored-by: David Ittah <[email protected]>
…I/catalyst into cosinewindow_catalyst_support
…I/catalyst into cosinewindow_catalyst_support
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1215 +/- ##
=======================================
Coverage 97.96% 97.96%
=======================================
Files 77 77
Lines 11244 11244
Branches 967 967
=======================================
Hits 11015 11015
Misses 180 180
Partials 49 49 ☔ View full report in Codecov by Sentry. |
Hello. You may have forgotten to update the changelog!
|
Thanks for adding this, I will take a closer look in a bit, but first @mlxd , what's the guideline on large batches of xfailed python tests like this? Do we prefer For context, @willjmax is going over the core PL transforms and testing whether qjit works with them or not. If they don't work, a reason is noted in this testing file. Edit: discussed this; if we know something fails, but it shouldn't, we should always aim to mark it xfail. |
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 adding the tests 💯 Some questions from me
@@ -15,4 +15,4 @@ pennylane=0.39.0.dev30 | |||
# 'runtime/Makefile' and at all GitHub workflows, using the exact | |||
# commit hash corresponding to the merged PR that implements the | |||
# desired feature. | |||
lightning=0.39.0-dev38 | |||
lightning=0.39.0-dev38 |
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.
Unsure why this changed; we leave an empty line at the end of all files.
assert expected_shape == observed_shape | ||
|
||
|
||
@pytest.mark.xfail(reason="Not JAX JIT compatible") |
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.
In cases where jax jit fails but qjit succeeds, I think we should compare qjit with plain pennylane.
assert expected_shape == observed_shape | ||
|
||
|
||
@pytest.mark.xfail(reason="cond does not work with qml.RY") |
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.
Is this a pennylane error or qjit error?
assert expected_shape == observed_shape | ||
|
||
|
||
@pytest.mark.xfail(reason="cond does not work with qml.RY") |
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.
ditto
assert expected_shape == observed_shape | ||
|
||
|
||
@pytest.mark.xfail( | ||
reason="ValueError: Eagerly computing the adjoint (lazy=False) is only supported on single operators." |
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.
Is this a pennylane error or a qjit error?
assert expected_shape == observed_shape | ||
|
||
|
||
@pytest.mark.xfail(reason="Something in pyzx not JAX JIT compatible.") |
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.
Does qjit work on this and jax jit fail, or both qjit and jax jit fail?
assert expected_shape == observed_shape | ||
|
||
|
||
@pytest.mark.xfail(reason="JIT and QJIT return different values.") |
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.
How about qjit and plain pennylane?
@@ -121,44 +166,896 @@ def qfunc(data, x, weights): | |||
assert expected_shape == observed_shape | |||
|
|||
|
|||
def test_split_non_commuting(backend): | |||
"""Test split non commuting""" | |||
@pytest.mark.xfail(reason="JIT and QJIT give different results.") |
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.
How about qjit and plain pennylane?
class TestMitigate: | ||
"""Test error mitigation transforms""" | ||
|
||
@pytest.mark.xfail(reason="JIT and QJIT return different values.") |
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.
How about qjit and plain pennylane?
_, observed_shape = jax.tree_util.tree_flatten(observed) | ||
assert expected_shape == observed_shape | ||
|
||
@pytest.mark.xfail(reason="JIT and QJIT return different values.") |
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.
How about qjit and plain pennylane?
@@ -15,4 +15,4 @@ pennylane=0.39.0.dev30 | |||
# 'runtime/Makefile' and at all GitHub workflows, using the exact | |||
# commit hash corresponding to the merged PR that implements the | |||
# desired feature. | |||
lightning=0.39.0-dev38 | |||
lightning=0.39.0-dev38 |
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.
lightning=0.39.0-dev38 | |
lightning=0.39.0-dev38 | |
# pylint: disable=unnecessary-lambda-assignment | ||
# pylint: disable=too-many-lines | ||
# pylint: disable=line-too-long |
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.
Could be combined together.
assert expected_shape == observed_shape | ||
|
||
|
||
@pytest.mark.xfail(reason="cond does not work with qml.RY") |
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.
Will this work with applying qml.RY
via a function, i.e., using callable for true
branch? 🤔
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 guess #449 really needs to be fixed! I will take a look.
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 fix is likely non-trivial, so no need to wait for it.
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.
assert expected_shape == observed_shape | ||
|
||
|
||
@pytest.mark.xfail(reason="ValueError on Lightning") |
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.
What is this error?
This PR adds unit tests for all PennyLane transforms. This PR addresses [sc-72626].