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

2115 pauli twirling callibration of expectation estimation shadow needs continue #2116

Conversation

bdg221
Copy link
Collaborator

@bdg221 bdg221 commented Dec 8, 2023

fixes #2115

Added continue to the case where f_val from the pauli twirling calibration is None.

This now prevents an additional mean value being incorrectly included for a particular split.

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.

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7d05a9f) 98.21% compared to head (9384ced) 98.21%.
Report is 2 commits behind head on master.

Files Patch % Lines
mitiq/shadows/classical_postprocessing.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2116   +/-   ##
=======================================
  Coverage   98.21%   98.21%           
=======================================
  Files          87       87           
  Lines        4135     4135           
=======================================
  Hits         4061     4061           
  Misses         74       74           

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

Comment on lines 370 to 371
means.append(0.0)
continue
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
means.append(0.0)
continue
product = 0.0

Is this equivalent? If so, I would suggest using product = 0.0 since it makes the workflow more linear and easier to debug in the future.

If my suggestion doesn't work, I am fine with continue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That makes sense. Should a similar thing be done with the "else" on line 374 (when there aren't any matches between the measurement_outcome and the pauli_str expected outcomes.) That else would also include a product = 0.0 and at the end of the for loop do a single means.append(np.sum(product) / len(idxes))?

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.

Thanks @bdg221 !

@natestemen natestemen merged commit 36138d5 into unitaryfund:master Dec 18, 2023
14 of 16 checks passed
andreamari added a commit that referenced this pull request Dec 20, 2023
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.

pauli twirling callibration of expectation_estimation_shadow needs continue
3 participants