-
Notifications
You must be signed in to change notification settings - Fork 612
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
Add shots.bins() generator method #5476
Add shots.bins() generator method #5476
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5476 +/- ##
==========================================
- Coverage 99.67% 99.66% -0.01%
==========================================
Files 406 406
Lines 37881 37637 -244
==========================================
- Hits 37758 37512 -246
- Misses 123 125 +2 ☔ View full report in Codecov by Sentry. |
eaf183d
to
bbae7d4
Compare
bbae7d4
to
0ca1341
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the PRNG key failures:
FAILED tests/devices/qubit/test_sampling.py::TestBroadcastingPRNG::test_nonsample_measure_shot_vector[measurement1-expected1-shots3] - assert False
+ where False = <function allclose at 0x7f8312da2e70>(Array([-1. , 1. , -0.0238], dtype=float64), array([-1, 1, 0]), atol=0.01)
+ where <function allclose at 0x7f8312da2e70> = np.allclose
FAILED tests/devices/qubit/test_sampling.py::TestBroadcastingPRNG::test_nonsample_measure_shot_vector[measurement1-expected1-shots4] - assert False
+ where False = <function allclose at 0x7f8312da2e70>(Array([-1. , 1. , -0.0163], dtype=float64), array([-1, 1, 0]), atol=0.01)
+ where <function allclose at 0x7f8312da2e70> = np.allclose
= 2 failed, 1066 passed, 108 skipped, 15 xfailed, 88 warnings in 737.38s (0:12:17) =
I think we will just have to change the test a little to accept the new results. Basically, we should expect the samples with shot vectors to be slightly different now, as sampling 2 then sampling 1 will give different values then sampling 3 at the same time, especially when PRNG keys are involved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Happy to finally have that alternate way of calculating shot vectors implemented.
My one comment was purely stylistic.
Requested a second review from the team. |
There was a problem hiding this 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, thanks @Tarun-Kumar07 ! 🎉
I had two small comments, one of which we briefly should discuss with @albi3ro .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Tarun-Kumar07 💯
Context:
The current approach for sampling multiple shots involves handling shot vectors by sampling multiple times and processing each shot independently. However, a more efficient strategy would be to sample all shots at once and then process each bin separately. The
shots.bins()
method facilitates this by providing the lower and upper bounds for each bin.Description of the Change:
shots.bins()
method generates a sequence of tuples, where each tuple represents the lower and upper bounds of a bin._measure_with_samples_diagonalizing_gates
sample once with total number of shots and useshots.bins()
to process each bin separatelyBenefits:
This method is particularly useful for processing shot quantities in a partitioned manner, as it allows for the separate handling of each bin's range
Possible Drawbacks:
Related GitHub Issues:
Fixes #5433