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

[Classical Shadows 1] utils for shadows and unit test #1907

Merged
merged 33 commits into from
Jul 31, 2023

Conversation

Min-Li
Copy link
Contributor

@Min-Li Min-Li commented Jul 9, 2023

Description

utils for shadows and unit test from #1899


License

  • I license this contribution under the terms of the GNU GPL, version 3 and grant Unitary Fund the right to provide additional permissions as described in section 7 of the GNU GPL, version 3.

Before opening the PR, please ensure you have completed the following where appropriate.

@Min-Li Min-Li requested a review from nathanshammah July 9, 2023 23:04
@Min-Li Min-Li self-assigned this Jul 9, 2023
@github-actions
Copy link

github-actions bot commented Jul 9, 2023

Binder 👈 Launch a binder notebook on branch Min-Li/mitiq/shadows/utils

@codecov
Copy link

codecov bot commented Jul 9, 2023

Codecov Report

Merging #1907 (9134067) into master (5b8a72b) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 9134067 differs from pull request most recent head a6c5793. Consider uploading reports for the commit a6c5793 to get more accurate results

@@            Coverage Diff             @@
##           master    #1907      +/-   ##
==========================================
- Coverage   98.47%   98.46%   -0.02%     
==========================================
  Files          79       81       +2     
  Lines        3682     3717      +35     
==========================================
+ Hits         3626     3660      +34     
- Misses         56       57       +1     
Files Changed Coverage Δ
mitiq/shadows/shadows_utils.py 100.00% <100.00%> (ø)
mitiq/shadows/test/test_shadows_utils.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@Min-Li Min-Li changed the title [Classical Shadows] utils for shadows and unit test [Classical Shadows 1f] utils for shadows and unit test Jul 10, 2023
@Min-Li Min-Li changed the title [Classical Shadows 1f] utils for shadows and unit test [Classical Shadows 1] utils for shadows and unit test Jul 10, 2023
@Min-Li Min-Li changed the title [Classical Shadows 1] utils for shadows and unit test [Classical Shadows] utils for shadows and unit test Jul 12, 2023
@natestemen
Copy link
Member

It looks like some of the tests are failing on python 3.8 due to a failure in subscripting np.ndarray. As in the other PR (#1908 (comment)) we should try to use numpy.typing for type hints.

Copy link
Member

@andreamari andreamari left a comment

Choose a reason for hiding this comment

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

Minor comments/suggestions

mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
Copy link
Member

@nathanshammah nathanshammah left a comment

Choose a reason for hiding this comment

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

Thanks @Min-Li, looks good to me, besides @andreamari's comments.

Min-Li and others added 3 commits July 17, 2023 10:45
Co-authored-by: Andrea Mari <andreamari84@gmail.com>
Co-authored-by: Andrea Mari <andreamari84@gmail.com>
@Min-Li Min-Li mentioned this pull request Jul 18, 2023
6 tasks
Copy link
Member

@natestemen natestemen left a comment

Choose a reason for hiding this comment

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

Nice work. Just some small clean up items from me.

mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
Min-Li and others added 3 commits July 21, 2023 09:33
Co-authored-by: nate stemen <nate@stemen.email>
Co-authored-by: nate stemen <nate@stemen.email>
…urements_opts_expectation_bound` as a standalone function
@Min-Li Min-Li changed the title [Classical Shadows] utils for shadows and unit test [Classical Shadows 1] utils for shadows and unit test Jul 23, 2023
Copy link
Member

@andreamari andreamari left a comment

Choose a reason for hiding this comment

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

From my side, LGTM! Thanks Min!

Copy link
Member

@natestemen natestemen left a comment

Choose a reason for hiding this comment

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

Looking good! A few minor questions/comments.

mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/test/test_shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
mitiq/shadows/shadows_utils.py Outdated Show resolved Hide resolved
@Min-Li Min-Li requested a review from natestemen July 26, 2023 23:36
@Min-Li Min-Li requested a review from natestemen July 28, 2023 16:26
@natestemen natestemen merged commit 9e8c619 into unitaryfund:master Jul 31, 2023
18 of 19 checks passed
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.

4 participants