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

Trim down MCM tests. #754

Merged
merged 13 commits into from
Jun 10, 2024
Merged

Trim down MCM tests. #754

merged 13 commits into from
Jun 10, 2024

Conversation

vincentmr
Copy link
Contributor

@vincentmr vincentmr commented Jun 6, 2024

Before submitting

Please complete the following checklist when submitting a PR:

  • All new features must include a unit test.
    If you've fixed a bug or added code that should be tested, add a test to the
    tests directory!

  • All new functions and code must be clearly commented and documented.
    If you do make documentation changes, make sure that the docs build and
    render correctly by running make docs.

  • Ensure that the test suite passes, by running make test.

  • Add a new entry to the .github/CHANGELOG.md file, summarizing the
    change, and including a link back to the PR.

  • Ensure that code is properly formatted by running make format.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


Context:
MCM tests are slow and dominate the CI run time. New tests have been incorporated as new features came in without rationalizing the test suite as a whole. There are redundant or overly parametrized tests as a results.

Description of the Change:
Combine a bunch of tests into test_simple_dynamic_circuit which tests all possible measurements are handle correctly by dynamic_one_shot post-processing. This needs not be parametrized over reset since it does not show up in post-processing.

Combine some tests into test_multiple_measurements_and_reset which checks that multiple measurements are handle correctly and that reset is properly applied while getting to the final state.

Combine all composite MCM measurements in test_composite_mcms.

test_counts_return_type remains as is.

Benefits:
Faster CI.

Possible Drawbacks:

Related GitHub Issues:
[sc-65255]

Copy link

codecov bot commented Jun 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.71%. Comparing base (9b933a2) to head (3d63679).
Report is 86 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #754      +/-   ##
==========================================
- Coverage   93.40%   92.71%   -0.70%     
==========================================
  Files         120       15     -105     
  Lines       17462     1729   -15733     
==========================================
- Hits        16311     1603   -14708     
+ Misses       1151      126    -1025     

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

@vincentmr vincentmr changed the title Trim down tests. Trim down MCM tests. Jun 10, 2024
@vincentmr vincentmr marked this pull request as ready for review June 10, 2024 16:42
Copy link
Contributor

@AmintorDusko AmintorDusko 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 so far!

.github/workflows/tests_lkcpu_python.yml Outdated Show resolved Hide resolved
tests/test_native_mcm.py Outdated Show resolved Hide resolved
tests/test_native_mcm.py Outdated Show resolved Hide resolved
tests/test_native_mcm.py Outdated Show resolved Hide resolved
tests/test_native_mcm.py Outdated Show resolved Hide resolved
tests/test_native_mcm.py Outdated Show resolved Hide resolved
vincentmr and others added 4 commits June 10, 2024 15:22
Co-authored-by: Amintor Dusko <87949283+AmintorDusko@users.noreply.github.com>
Co-authored-by: Amintor Dusko <87949283+AmintorDusko@users.noreply.github.com>
Co-authored-by: Amintor Dusko <87949283+AmintorDusko@users.noreply.github.com>
Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

Thank you @vincentmr. Great PR!

@AmintorDusko AmintorDusko requested a review from a team June 10, 2024 20:23
Copy link
Member

@multiphaseCFD multiphaseCFD left a comment

Choose a reason for hiding this comment

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

Thanks @vincentmr ! Just a few question but happy to approve.

@vincentmr vincentmr merged commit 547de08 into master Jun 10, 2024
65 of 66 checks passed
@vincentmr vincentmr deleted the reduce_mcm_tests branch June 10, 2024 21:23
multiphaseCFD pushed a commit that referenced this pull request Jun 10, 2024
Please complete the following checklist when submitting a PR:

- [x] All new features must include a unit test.
If you've fixed a bug or added code that should be tested, add a test to
the
      [`tests`](../tests) directory!

- [x] All new functions and code must be clearly commented and
documented.
If you do make documentation changes, make sure that the docs build and
      render correctly by running `make docs`.

- [x] Ensure that the test suite passes, by running `make test`.

- [x] Add a new entry to the `.github/CHANGELOG.md` file, summarizing
the
      change, and including a link back to the PR.

- [x] Ensure that code is properly formatted by running `make format`.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.

------------------------------------------------------------------------------------------------------------

**Context:**
MCM tests are slow and dominate the CI run time. New tests have been
incorporated as new features came in without rationalizing the test
suite as a whole. There are redundant or overly parametrized tests as a
results.

**Description of the Change:**
Combine a bunch of tests into `test_simple_dynamic_circuit` which tests
all possible measurements are handle correctly by `dynamic_one_shot`
post-processing. This needs not be parametrized over `reset` since it
does not show up in post-processing.

Combine some tests into `test_multiple_measurements_and_reset` which
checks that multiple measurements are handle correctly and that `reset`
is properly applied while getting to the final state.

Combine all composite MCM measurements in `test_composite_mcms`.

`test_counts_return_type` remains as is.

**Benefits:**
Faster CI.

**Possible Drawbacks:**

**Related GitHub Issues:**
[sc-65255]

---------

Co-authored-by: ringo-but-quantum <github-ringo-but-quantum@xanadu.ai>
Co-authored-by: Amintor Dusko <87949283+AmintorDusko@users.noreply.github.com>
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.

4 participants