-
Couldn't load subscription status.
- Fork 706
All SampleMeasurement processes include a dtype optional parameter
#8214
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
Conversation
…o precision_optional_parameter
…o precision_optional_parameter
|
Hello. You may have forgotten to update the changelog!
|
…o SampleMeasurements_dtype_param
…nyLaneAI/pennylane into SampleMeasurements_dtype_param
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.
Can we add this to MeasurementProcess as a whole instead?
@albi3ro Yes, that was already in the commit that I was going to push : ) GitHub accidentally tagged our team automatically, but this PR is still not ready for review. Sorry for the notification! |
|
Is this already ready for review? |
No, please look at this PR instead (which only adds this new optional parameter to |
…nyLaneAI/pennylane into SampleMeasurements_dtype_param
Co-authored-by: Christina Lee <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8214 +/- ##
========================================
Coverage 99.39% 99.40%
========================================
Files 551 552 +1
Lines 57223 57375 +152
========================================
+ Hits 56879 57031 +152
Misses 344 344 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
pylint seems to be catching some mismatches 🤔 |
|
@JerryChen97 Please ignore this PR as it is not review-ready : ) |
…o SampleMeasurements_dtype_param
…o SampleMeasurements_dtype_param
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.
@qml.qnode(qml.device('default.qubit'))
def c():
return qml.expval(qml.X(0), dtype=np.float16)
c().dtype
dtype('float64')
Also need to convert in analytic mode.
But for analytic processing, we may actually need to go into the relevant device code and add some conversions there too.
|
@albi3ro Exactly, that's why I mentioned in the doc that the Also, it seems to me that the main motivation for this was a (future) speed-up improvement, which is particularly relevant when samples are used. Do we expect any major performance benefit in providing a |
| powers_of_two = 2 ** math.arange(num_wires)[::-1] | ||
| indices = samples @ powers_of_two | ||
| indices = math.array(indices) # Add np.array here for Jax support. | ||
| indices = math.array(indices, dtype=int) # Add np.array here for Jax support. |
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 observables multiplied by complex coefficients, indices might be created as a complex array, but it should always be integer
If it's too much work to get the feature done right now, then we can just backlog it and come back to this later. Getting the feature in for |
|
@albi3ro Sounds good. I will 'freeze' this PR and backlog the story then. I will still add a 1-line change in a separate PR as I realized that in case of a complex coefficient times an observable, the indices created with |
Context: In this PR, we extend the usage of the
dtypeoptional parameter to all measurement processes inheriting fromSampleMeasurement(except forqml.countsas this always returns adictwith integer keys and values):qml.expvalqml.varqml.probsqml.sample(already implemented in this PR)Description of the Change: As above.
Benefits: More flexibility and symbolic (imaginary) speed up improvement.
Possible Drawbacks: None that I can think of.
Related GitHub Issues: None.
[sc-98735]