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

transforms.insert: Update docs, improve decomposition #5681

Merged
merged 7 commits into from
May 14, 2024
Merged

Conversation

dwierichs
Copy link
Contributor

Context:
The first example in the docs of qml.transforms.insert was reported to not be working here.
Other examples also do not work.

Description of the Change:
This PR changes the docs examples to work.

It also replicates the change in #5424 to use devices.preprocess.decompose instead of tape.expand, adding support for non-commuting measurements (which is unrelated to the transform anyways), and removing the need for a custom error to be raised.

Benefits:
Docs examples work.
Non-commuting measurements support.

Possible Drawbacks:
Similar to #5424, this introduces cross-dependency to the devices module.

Related GitHub Issues:

Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@dwierichs dwierichs added the review-ready 👌 PRs which are ready for review by someone from the core team. label May 11, 2024
Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @dwierichs!

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.

Looks like this will still be waiting on all the autoray issues, but still gets an approval from me :)

@dwierichs dwierichs enabled auto-merge (squash) May 14, 2024 14:52
@dwierichs dwierichs added merge-ready ✔️ All tests pass and the PR is ready to be merged. and removed review-ready 👌 PRs which are ready for review by someone from the core team. labels May 14, 2024
Copy link

codecov bot commented May 14, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 99.67%. Comparing base (be35a7b) to head (ac2877e).

Files Patch % Lines
pennylane/transforms/insert_ops.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5681      +/-   ##
==========================================
- Coverage   99.68%   99.67%   -0.01%     
==========================================
  Files         414      414              
  Lines       38814    38527     -287     
==========================================
- Hits        38691    38402     -289     
- Misses        123      125       +2     

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

@dwierichs dwierichs merged commit 57e8d93 into master May 14, 2024
37 of 38 checks passed
@dwierichs dwierichs deleted the insert-ex-fix branch May 14, 2024 19:42
Shiro-Raven pushed a commit that referenced this pull request May 15, 2024
**Context:**
The first example in the docs of `qml.transforms.insert` was reported to
not be working
[here](https://discuss.pennylane.ai/t/qml-device-error-with-qiskit-aer/4556/9).
Other examples also do not work.

**Description of the Change:**
This PR changes the docs examples to work.

It also replicates the change in #5424 to use
`devices.preprocess.decompose` instead of `tape.expand`, adding support
for non-commuting measurements (which is unrelated to the transform
anyways), and removing the need for a custom error to be raised.

**Benefits:**
Docs examples work.
Non-commuting measurements support.

**Possible Drawbacks:**
Similar to #5424, this introduces cross-dependency to the devices
module.

**Related GitHub Issues:**
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.

3 participants