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

Update shot_adaptive to not mutate device shots #4599

Merged
merged 5 commits into from
Sep 19, 2023

Conversation

timmysilv
Copy link
Contributor

Context:
shot_adaptive.py would change device wires, run a QNode with the device, then change the wires back. We can just run the QNode with different shots without changing the device!

Description of the Change:
Instead of changing device shots, pass shots=new_shots as a kwarg to any QNode execution.

I also removed a silly test for something hacky that users probably should not even be able to do... it's not that hard to add @qml.qnode(qml.device("default.qubit", shots=1000)) to the top of any function : )

Benefits:

  • It will work on the new device API without any changes (I ran the tests locally with DQ2, worked perfectly)
  • No more unnecessary try-finally blocks
  • Not mutating object properties is always cool and wise

@timmysilv timmysilv requested a review from a team September 15, 2023 03:02
@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.

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01% ⚠️

Comparison is base (69475a2) 99.64% compared to head (17db120) 99.64%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4599      +/-   ##
==========================================
- Coverage   99.64%   99.64%   -0.01%     
==========================================
  Files         375      375              
  Lines       33358    33342      -16     
==========================================
- Hits        33239    33222      -17     
- Misses        119      120       +1     
Files Changed Coverage Δ
pennylane/optimize/shot_adaptive.py 99.21% <100.00%> (-0.79%) ⬇️

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

@timmysilv
Copy link
Contributor Author

[sc-44399]

@timmysilv timmysilv changed the title Update shot_adaptive to not mutate device wires Update shot_adaptive to not mutate device shots Sep 18, 2023
@timmysilv timmysilv requested a review from a team September 18, 2023 18:03
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 good. Thanks!

@timmysilv timmysilv enabled auto-merge (squash) September 18, 2023 20:18
@timmysilv timmysilv merged commit 5350747 into master Sep 19, 2023
39 checks passed
@timmysilv timmysilv deleted the shot-adaptive-shots-kwarg branch September 19, 2023 15:42
mudit2812 pushed a commit that referenced this pull request Sep 19, 2023
**Context:**
`shot_adaptive.py` would change device wires, run a QNode with the
device, then change the wires back. We can just run the QNode with
different shots without changing the device!

**Description of the Change:**
Instead of changing device shots, pass `shots=new_shots` as a kwarg to
any QNode execution.

I also removed a silly test for something hacky that users probably
should not even be able to do... it's not that hard to add
`@qml.qnode(qml.device("default.qubit", shots=1000))` to the top of any
function : )

**Benefits:**
- It will work on the new device API without any changes (I ran the
tests locally with DQ2, worked perfectly)
- No more unnecessary try-finally blocks
- Not mutating object properties is always cool and wise
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