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

remove inverse support; add adjoint support; state-prep op cleanup #130

Merged
merged 12 commits into from
Feb 16, 2023

Conversation

timmysilv
Copy link
Contributor

@timmysilv timmysilv commented Feb 11, 2023

As the title says. op.inv() and op.inverse are removed from Operators in PennyLane 0.29, so this plugin needs updating accordingly. To summarize how I do it:

  • All natively supported ops/obs are in a dict called _base_op/obs_map
  • I dynamically copy the list, surrounding the names with Adjoint(...) (if the op exists and isn't already a natively supported adjoint op) and call the _op/obs_map. They're separate because the QSim devices did not support inverse ops so this will make it easy to preserve that assumption.
  • The ops/obs list exposed to the pennylane supports_op/obs method now includes those Adjoint(...) ops/obs. No custom device.capabilities["supports_inverse_operation"] changes are needed because adjoint op names will be in the dict (rather than a hidden .inv property)
  • The qsim(h) devices only expose the base ops (ie. without the Adjoint counterparts) to the pennylane supports_obs/ops thanks to the dict separation described above

The plugin also did lots of custom handling of StatePrep operators that will now be handled by the operators themselves, so I have removed that code. The custom handling had its own errors so tests were failing as they were, which is why I changed them in this PR. Both this and the inverse changes should happen in the same PR to update to the latest pennylane without breaking anything.

Note that because StatePrep.state_vector did not exist before PL 0.29, the version in setup.py will need to be bumped before this plugin is released.

@codecov
Copy link

codecov bot commented Feb 11, 2023

Codecov Report

Merging #130 (ce6ccc3) into master (4c09d20) will decrease coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
- Coverage   99.23%   99.14%   -0.09%     
==========================================
  Files           8        8              
  Lines         392      352      -40     
==========================================
- Hits          389      349      -40     
  Misses          3        3              
Impacted Files Coverage Δ
pennylane_cirq/pasqal_device.py 100.00% <ø> (ø)
pennylane_cirq/cirq_device.py 98.96% <100.00%> (-0.07%) ⬇️
pennylane_cirq/cirq_operation.py 100.00% <100.00%> (ø)
pennylane_cirq/qsim_device.py 96.87% <100.00%> (ø)
pennylane_cirq/simulator_device.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@timmysilv timmysilv changed the title remove inverse support; add adjoint support remove inverse support; add adjoint support; state-prep op cleanup Feb 13, 2023
@timmysilv
Copy link
Contributor Author

[sc-33452]

Copy link

@AlbertMitjans AlbertMitjans left a comment

Choose a reason for hiding this comment

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

Amazing job! Left some comments, but it is good to go 💯 🔥

pennylane_cirq/cirq_device.py Outdated Show resolved Hide resolved
tests/test_apply.py Show resolved Hide resolved
tests/test_apply.py Show resolved Hide resolved
Copy link
Contributor

@lillian542 lillian542 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 to me!

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.

3 participants