-
Notifications
You must be signed in to change notification settings - Fork 66
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
Example added BasicSim
docs that shows default shots
#549
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #549 +/- ##
===========================================
- Coverage 100.00% 99.45% -0.55%
===========================================
Files 7 7
Lines 557 550 -7
===========================================
- Hits 557 547 -10
- Misses 0 3 +3 ☔ View full report in Codecov by Sentry. |
Hi @isaacdevlugt thanks for the PR! Just to clarify, when you say "hardcoded" do you mean that even if you specify e.g. |
Hey @austingmhuang! I mean the latter. 1024 is hardcoded as the default number (can be overridden).
|
Why don't we update |
@trbromley I think the idea is that when a User doesn't pass in a # of shots, there is supposed to be a warning that lets users know that analytic mode is not supported on the basicsim device and inform them that the circuit was ran with 1024 shots. Right now I think this warning is bugged so it isn't showing up but this is the way it's done when using the new QiskitDevice. Obviously we can just raise an error instead but I think this solution is a bit more consistent with how devices in pennylane generally work. |
In Qiskit, when you use this device / backend you’re hard-corded at
shots=1024
. I assume Qiskit users are aware of this. However, when I used this device in PennyLane, I ran this circuit not expecting it to work because I didn’t specify shots.I had to go digging into the source code to figure out what was going on. As a PennyLane user, I found this confusing (does that matter? lol). Anyway, a simple addition to the docs seems fine to me.