Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

Correct QVFitter confidence level threshold to 0.977 #482

Merged
merged 12 commits into from
Sep 2, 2020
Merged

Correct QVFitter confidence level threshold to 0.977 #482

merged 12 commits into from
Sep 2, 2020

Conversation

HuangJunye
Copy link
Contributor

@HuangJunye HuangJunye commented Aug 25, 2020

Correct QVFitter confidence level threshold to 0.977 corresponding to z = 2 as defined by the QV paper Algorithm 1

Summary

Fixes #473 and #481 :

  1. Added new methods calc_z_value and calc_confidence_level.
  2. Correct the confidence level threshold to calc_confidence_level(z_value=2).
  3. Store confidence level even when hmean < 2/3.
  4. Raises a warning if sigma=0 in calc_z_value
  5. Added explanations for how to calculate statistics based on binomial distribution in calc_statistics()

Details and comments

@HuangJunye
Copy link
Contributor Author

Suggest reviewers @dcmckayibm @levbishop. CC: @oliverdial.

@ShellyGarion
Copy link
Contributor

Hi @HuangJunye - thank you for your contribution. Please note that if you would like your code to get merged, then you should fix the minor travis errors. Thanks!

@HuangJunye
Copy link
Contributor Author

@ShellyGarion Sorry about that. This is the first time I am making a real PR haha. I removed the trailing white spaces which I think are the reasons for Travis errors. Still waiting for the build to finish but the build that failed has passed now.

Shall I add add anything to release note?

@HuangJunye
Copy link
Contributor Author

Made a minor change to store confidence level even when hmean < 2/3 based on the comment from @oliverdial #483

@HuangJunye
Copy link
Contributor Author

I realise what I did is not what @oliverdial asked for. He wants to store the confidence interval not confidence level. The QVFitter doesn’t calculate or store confidence interval. It calculate the confidence level for hmean > 2/3. I think it doesn’t harm to store confidence level for hmean < 2/3, but adding a feature To calculate confidence interval is probably another PR.

@oliverdial
Copy link

Actually, I misread the code -- I thought in some cases sigma was not stored, but it always is (with this the CI is easy to calculate). But saving the confidence level as well is still nice -- thanks!

@ShellyGarion
Copy link
Contributor

Shall I add add anything to release note?

After you verify that this PR is correct and fits the theory of the QV paper, then you may also add release notes as you suggested.

@HuangJunye
Copy link
Contributor Author

HuangJunye commented Aug 26, 2020

I tested with the quantum volume tutorial and textbook chapter. It behaves exactly as before the changes. I think it’s correct but the best would be getting @dcmckayibm or @levbishop to review the PR.

@HuangJunye
Copy link
Contributor Author

Raises ValueError when sigma = 0 and added explanations for how to calculate statistics based on binomial distribution in calc_statistics() (addresses #481).

@HuangJunye
Copy link
Contributor Author

@dcmckayibm I have added the error if, can you please review the code? Thank you.

@HuangJunye
Copy link
Contributor Author

Thanks @dcmckayibm for your approval!

@ShellyGarion
Copy link
Contributor

Thanks @HuangJunye for your contribution!

@HuangJunye
Copy link
Contributor Author

Thank you @ShellyGarion. I am really excited to contribute my first ever PR to Qiskit!

@lazyoracle
Copy link

:shipit:

@chriseclectic chriseclectic added the Changelog: Bugfix Include in the Fixed section of the changelog label Sep 2, 2020
@chriseclectic chriseclectic merged commit 4c0369f into qiskit-community:master Sep 2, 2020
@HuangJunye
Copy link
Contributor Author

@chriseclectic Thanks for merging the PR!!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog: Bugfix Include in the Fixed section of the changelog stable-backport-potential
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QV uses wrong threshold for statistical test
7 participants