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

Transform on devices and update insert #4667

Merged
merged 17 commits into from
Oct 13, 2023
Merged

Transform on devices and update insert #4667

merged 17 commits into from
Oct 13, 2023

Conversation

rmoyard
Copy link
Contributor

@rmoyard rmoyard commented Oct 12, 2023

Description of the Change:

  • Transforms can now be added to the preprocessing of device. dev = transform(dev)
  • Insert is updated to the new transform system

Benefits:

Possible Drawbacks:
Default mixed is not a new generation device, the main use cases of applying insert on a device is for default mixed (adding noise).

  • Add a special patch in transform dispatcher in the case of default mixed (to be removed in the future)

Related GitHub Issues:

@github-actions
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.

@rmoyard rmoyard changed the title Transform on devices Transform on devices and update insert Oct 12, 2023
@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (eafabe4) 99.65% compared to head (cae0fbc) 99.65%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4667   +/-   ##
=======================================
  Coverage   99.65%   99.65%           
=======================================
  Files         376      376           
  Lines       33678    33702   +24     
=======================================
+ Hits        33561    33585   +24     
  Misses        117      117           
Files Coverage Δ
pennylane/transforms/core/transform_dispatcher.py 100.00% <100.00%> (ø)
pennylane/transforms/insert_ops.py 100.00% <100.00%> (ø)
pennylane/transforms/qfunc_transforms.py 90.00% <ø> (-3.16%) ⬇️

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

@rmoyard
Copy link
Contributor Author

rmoyard commented Oct 12, 2023

[sc-43680]

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 we've removed some test coverage from qfunc_transform, but I'm fine with overriding that with in this case. qfunc_transform is slated for deprecation, and its the fault of qfunc_transform for not having proper unit tests.

Changelog and unit test for qml.Device, and I'd be happy to approve :)

Copy link
Contributor

@timmysilv timmysilv left a comment

Choose a reason for hiding this comment

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

changes look good, just some little comments.

i don't remember if we had already changed some transforms in the last release, but will we need to announce the breaking changes re: how to decorate a qnode, particularly with @partial?

pennylane/transforms/insert_ops.py Outdated Show resolved Hide resolved
@rmoyard
Copy link
Contributor Author

rmoyard commented Oct 13, 2023

@timmysilv I will update the transforms changelog during feature freeze to make sure all details are covered such as the @partial

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.

👍 🚀

@rmoyard rmoyard enabled auto-merge (squash) October 13, 2023 16:02
Copy link
Contributor

@timmysilv timmysilv left a comment

Choose a reason for hiding this comment

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

I'd like to allow for adding transforms to the front (rather than the back) of a program like this, but we can do that in another PR. For example, it would be useful if you want to decompose operators in a custom way before the device decomposition occurs (see what is being done in #4646 - it can't use this until support is added).

@rmoyard rmoyard merged commit 2f1ceb3 into master Oct 13, 2023
39 checks passed
@rmoyard rmoyard deleted the transform_device branch October 13, 2023 17:54
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