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

add process_counts method to SampleMeasurement #4941

Merged
merged 5 commits into from Dec 12, 2023
Merged

Conversation

albi3ro
Copy link
Contributor

@albi3ro albi3ro commented Dec 12, 2023

This PR adds a process_counts method to SampleMeasurement. This method is not implemented by anything and only raises a NotImplementedError.

This method is to make it easier to support plugins and remote devices that prefer producing and sending counts histograms instead of the raw samples.

@albi3ro albi3ro marked this pull request as ready for review December 12, 2023 15:58
@albi3ro albi3ro requested a review from a team December 12, 2023 15:58
Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

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

Small fixes included, but otherwise looks good :)

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/measurements/__init__.py Outdated Show resolved Hide resolved
@albi3ro
Copy link
Contributor Author

albi3ro commented Dec 12, 2023

[sc-51501]

albi3ro and others added 2 commits December 12, 2023 12:07
Co-authored-by: Matthew Silverman <matthews@xanadu.ai>
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
@albi3ro albi3ro enabled auto-merge (squash) December 12, 2023 17:08
Copy link

codecov bot commented Dec 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4942327) 99.66% compared to head (9b95cd1) 99.65%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4941      +/-   ##
==========================================
- Coverage   99.66%   99.65%   -0.02%     
==========================================
  Files         388      388              
  Lines       35360    35096     -264     
==========================================
- Hits        35243    34976     -267     
- Misses        117      120       +3     

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

@albi3ro albi3ro merged commit a05ae28 into master Dec 12, 2023
35 checks passed
@albi3ro albi3ro deleted the process_counts branch December 12, 2023 18:53
mudit2812 added a commit that referenced this pull request Jan 19, 2024
This PR adds a `process_counts` method to `SampleMeasurement`. This
method is not implemented by anything and only raises a
`NotImplementedError`.

This method is to make it easier to support plugins and remote devices
that prefer producing and sending counts histograms instead of the raw
samples.

---------

Co-authored-by: Matthew Silverman <matthews@xanadu.ai>
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
albi3ro added a commit that referenced this pull request Mar 12, 2024
…5256)

**Context:**
Under #4941 abstract method `process_counts` was added to
`SampleMeasurement`. This method provides support to process counts
dictionary produced by external systems.

**Description of the Change:**
* Implement `process_counts` in `ExpectationMP`, `VarianceMP` and
`CountsMP`.
* While implementing `process_counts` in `CountsMP` some common logic
was extracted from `ProbabilityMP` to `CountsMP`

**Benefits:**
The below methods won't throw `NotImplementedError` exception
* `ExpectationMP.process_counts`
* `VarianceMP.process_counts`
* `CountsMP.process_counts`

**Possible Drawbacks:**
* All classes which inherit from `SampleMeasurement` implement
`process_counts` except `SampleMP`.
Should I add an implementation for that ?
* After implementing `process_counts` in all child classes can we make
the method abstract similar to `process_samples` ? This might break some
tests where some classes inherit from `SampleMeasurement` as there
`process_counts` is not implemented
* It is assumed that `counts` dictionary is valid and caller is
responsible to call with valid dictionary. It has already been discussed
in this
[conversation](https://github.com/PennyLaneAI/pennylane/pull/4952/files/eb9c1ee81ea87c274a5ec094f90a0065d05fdc36#r1433117236)
and this is a design choice.

**Related GitHub Issues:**
This PR
* fixes #5249 
* fixes #5244 
* fixes #5241

***

**Further details**
I'm excited to be making my first open-source contribution with this PR
😄 .
As a newcomer to the community, I'm eager to learn and improve. 
Any feedback for enhancements would be greatly appreciated!

**Internal Shortcut Stories**
- [sc-57166]
- [sc-57274]
- [sc-57307]

---------

Co-authored-by: Christina Lee <chrissie.c.l@gmail.com>
Co-authored-by: Christina Lee <christina@xanadu.ai>
albi3ro added a commit that referenced this pull request Mar 25, 2024
**Context:**
Under #4941 abstract method `process_counts` was added
to`SampleMeasurement`. This method provides support to process counts
dictionary produced by external systems.

**Description of the Change:**
* Implement `process_counts` in `SampleMP`.
* Make `SampleMeasurement.process_counts` an abstract method

**Benefits:**
The method `SampleMP.process_counts` won't throw `NotImplementedError`
exception

**Possible Drawbacks:**

**Related GitHub Issues:**
Fixes #5371

---------

Co-authored-by: Astral Cai <astralcai@gmail.com>
Co-authored-by: Christina Lee <chrissie.c.l@gmail.com>
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.

None yet

3 participants