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

Test all operators with assert_valid, or fail with a reason #4978

Merged
merged 7 commits into from Jan 3, 2024

Conversation

timmysilv
Copy link
Contributor

Context:
Continuation of the work I started in #4922 (this is part 1 of the future work listed in that PR)

Description of the Change:

  1. Implement _flatten for BlockEncode and SpecialUnitary to pytreeify those types
  2. Organize tests/ops/functions/conftest.py to better parametrize tests. The parametrization fixtures are easier to make using the following:
  • _INSTANCES_TO_TEST is a list of operator instances that couldn't be auto-generated, but do pass validation
  • _INSTANCES_TO_FAIL is a list of tuples ((Operator, Type[Exception])) where the first element is an operator instance that will fail validation, and the second element is the exception type raised during validation. Ideally, it also comes with a comment explaining what's failing/why it should fail.

All test collection logic is now in conftest.py, making the tests much smaller and simpler. The only "extra" logic in the tests is the xfail I added for all qutrit ops before auto-generation. Now there are 3 tests:

  1. test_generated_list_of_ops, which auto-generates instances of every Operator type and asserts its validity
  2. test_explicit_list_of_ops, which tests the validity of manual operator instances
  3. test_explicit_list_of_failing_ops, which ensures that invalid operator instances fail to validate as expected

Benefits:
Every single pennylane operator (except for templates) is (in)validated in a single test file, and most of it is done automatically.

Possible Drawbacks:
Yet another test file that needs maintenance.

Copy link

codecov bot commented Jan 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f44f928) 99.67% compared to head (ae2916c) 99.67%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4978      +/-   ##
==========================================
- Coverage   99.67%   99.67%   -0.01%     
==========================================
  Files         392      392              
  Lines       35556    35303     -253     
==========================================
- Hits        35442    35188     -254     
- Misses        114      115       +1     

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

@timmysilv timmysilv requested a review from a team January 2, 2024 20:38
Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

I obviously like having a list of all the things we need to fix. Though I am distracted by wanting to fix all those issues immediately, the point of isn't to fix the issues, but just to keep a list of the things we need to fix.

Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

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

Looks good, but lets wait on merging this until after the RC-master merge and fixing the consequent IsingXX PickleError (or lack thereof).

@timmysilv timmysilv enabled auto-merge (squash) January 3, 2024 16:01
@timmysilv timmysilv merged commit 3fd3c0a into master Jan 3, 2024
35 checks passed
@timmysilv timmysilv deleted the test-all-ops branch January 3, 2024 16:32
mudit2812 pushed a commit that referenced this pull request Jan 19, 2024
**Context:**
Continuation of the work I started in #4922 (this is part 1 of the
future work listed in that PR)

**Description of the Change:**
1. Implement `_flatten` for `BlockEncode` and `SpecialUnitary` to
pytreeify those types
2. Organize `tests/ops/functions/conftest.py` to better parametrize
tests. The parametrization fixtures are easier to make using the
following:
- `_INSTANCES_TO_TEST` is a list of operator instances that couldn't be
auto-generated, but do pass validation
- `_INSTANCES_TO_FAIL` is a list of tuples (`(Operator,
Type[Exception])`) where the first element is an operator instance that
will fail validation, and the second element is the exception type
raised during validation. Ideally, it also comes with a comment
explaining what's failing/why it should fail.

All test collection logic is now in `conftest.py`, making the tests much
smaller and simpler. The only "extra" logic in the tests is the xfail I
added for all qutrit ops before auto-generation. Now there are 3 tests:
1. `test_generated_list_of_ops`, which auto-generates instances of every
Operator type and asserts its validity
2. `test_explicit_list_of_ops`, which tests the validity of manual
operator instances
3. `test_explicit_list_of_failing_ops`, which ensures that invalid
operator instances fail to validate as expected

**Benefits:**
Every single pennylane operator (except for templates) is (in)validated
in a single test file, and most of it is done automatically.

**Possible Drawbacks:**
Yet another test file that needs maintenance.

---------

Co-authored-by: Christina Lee <christina@xanadu.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants