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.

Sign up for GitHub

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

Jump to bottom

qml.Projector compatibility with new default qubit #4452

Merged
merged 23 commits into from Aug 16, 2023
Merged

qml.Projector compatibility with new default qubit #4452

merged 23 commits into from Aug 16, 2023

Conversation

BorjaRequena
Copy link
Collaborator

With the last release, qml.Projector lost its pickle-ability with the new dynamic type. This has affected some users (e.g. #4443) and makes it incompatible with the new default qubit multiprocessing. Testing the observable with the new device, I found a couple of additional points that had to be fixed for it to be fully compatible.

I have written a __reduce__ method for qml.Projector that allows it to be pickled. I have done so in a way that the object can be serialized and loaded keeping all its original attributes (including id). I have also changed the branching conditions for qml.Projector in measurements/expval.py and measurements/var.py as we had done originally for _device.py.

Fixes #4443

[sc-43218]

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023

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.

@codecov
Copy link

codecov bot commented Aug 8, 2023 •
edited

Codecov Report

Merging #4452 (c14c299) into master (ea624e0) will increase coverage by 0.00%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4452   +/-   ##
=======================================
  Coverage   99.71%   99.71%           
=======================================
  Files         376      376           
  Lines       34130    34134    +4     
=======================================
+ Hits        34034    34038    +4     
  Misses         96       96
Files Changed Coverage Δ
pennylane/drawer/drawable_layers.py 100.00% <ø> (ø)
pennylane/transforms/condition.py 100.00% <ø> (ø)
pennylane/_qubit_device.py 99.30% <100.00%> (+<0.01%) ⬆️
pennylane/measurements/expval.py 100.00% <100.00%> (ø)
pennylane/measurements/mid_measure.py 100.00% <100.00%> (ø)
pennylane/measurements/var.py 100.00% <100.00%> (ø)
pennylane/ops/qubit/observables.py 100.00% <100.00%> (ø)
pennylane/transforms/defer_measurements.py 100.00% <100.00%> (ø)

@BorjaRequena BorjaRequena mentioned this pull request Aug 8, 2023
Closed
@BorjaRequena BorjaRequena requested a review from a team August 11, 2023 19:37
timmysilv
timmysilv reviewed Aug 11, 2023
View reviewed changes
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.

this looks great! I'm wondering in light of a recent PR from @albi3ro for similar problems with adjoint, should we just change the typing of Projectors altogether? I'm personally fine to implement __reduce__ as you've done here, but we should probably be consistent with type-overloading across PL.

pennylane/measurements/expval.py Outdated Show resolved Hide resolved
pennylane/ops/qubit/observables.py Outdated Show resolved Hide resolved
pennylane/ops/qubit/observables.py Show resolved Hide resolved
tests/devices/experimental/test_default_qubit_2.py Outdated Show resolved Hide resolved
BorjaRequena and others added 2 commits August 11, 2023 16:53
@BorjaRequena @timmysilv
Fix __reduce__ docstring
7c066dd 
Co-authored-by: Matthew Silverman <matthews@xanadu.ai>
@BorjaRequena @timmysilv
Fix test
8d966b6 
Co-authored-by: Matthew Silverman <matthews@xanadu.ai>
@mudit2812 mudit2812 added this to the v0.32 milestone Aug 14, 2023
@timmysilv timmysilv mentioned this pull request Aug 14, 2023
Merged
@BorjaRequena
Copy link
Collaborator Author

BorjaRequena commented Aug 14, 2023 •
edited

Changing the output type of qml.Projector may cause issues with this dispatching in the braket plugin. If that is the case, we can split it into two functions instead of having a condition, which will be cleaner.

While checking this, we found that the Cirq plugin translates the qml.Projector into a product state projector, which may not properly handle some StateVectorProjectors with an entangled state. Probably worth looking into this 🤔

@BorjaRequena BorjaRequena requested review from timmysilv and a team August 15, 2023 13:13
timmysilv
timmysilv reviewed Aug 15, 2023
View reviewed changes
pennylane/ops/qubit/observables.py Outdated Show resolved Hide resolved
tests/ops/qubit/test_observables.py Outdated Show resolved Hide resolved
timmysilv
timmysilv approved these changes Aug 15, 2023
View reviewed changes
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.

fantastic!

albi3ro
albi3ro approved these changes Aug 15, 2023
View reviewed changes
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.

Thanks for figuring all these issues out 👍 🚀

@BorjaRequena BorjaRequena enabled auto-merge (squash) August 16, 2023 00:43
@BorjaRequena BorjaRequena merged commit 9ac4539 into master Aug 16, 2023
40 checks passed
@BorjaRequena BorjaRequena deleted the projector_pickle branch August 16, 2023 01:34
mlxd pushed a commit that referenced this pull request Aug 23, 2023
@BorjaRequena @timmysilv @mlxd
qml.Projector compatibility with new default qubit (#4452)
3def1a9 
* compatibility with DefaultQubit2

* update changelog

* testing tests

* extensive testing

* more tests

* Fix `__reduce__` docstring

Co-authored-by: Matthew Silverman <matthews@xanadu.ai>

* Fix test

Co-authored-by: Matthew Silverman <matthews@xanadu.ai>

* simplify typing

* formatting

* simplify measurement logic

* serialize without

* remove copy and extend serialization tests

* remove redundant inheritance

---------

Co-authored-by: Matthew Silverman <matthews@xanadu.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albi3ro albi3ro albi3ro approved these changes

@timmysilv timmysilv timmysilv approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.32
Development

Successfully merging this pull request may close these issues.

Make Projector instances pickleable
4 participants
@BorjaRequena @albi3ro @timmysilv @mudit2812