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

Feature BiRB #348

Merged
merged 19 commits into from
Sep 13, 2023
Merged

Feature BiRB #348

merged 19 commits into from
Sep 13, 2023

Conversation

jordanh6
Copy link
Contributor

@jordanh6 jordanh6 commented Sep 7, 2023

This PR primarily adds the code for running Binary RB (BiRB). Notable other changes:

  • RandomizedBenchmarking and SummaryStatistics classes have been modified to support BiRB's data type ('energies')
  • Added tutorials for BiRB and MRB with universal gate sets

@coreyostrove
Copy link
Contributor

Hey @jordanh6 , great work!

Regarding the failing unit tests, this is due to the installation failing on newer versions of python. This is a related to a dependency bug (not of our own doing) that was fixed in the latest release. If you merge 'develop' into this branch that should fix that issue and get the tests at least to the point of actually running again. Let me know if you'd prefer me to do so on your behalf.

@coreyostrove
Copy link
Contributor

There looks to be a change in ibmqcore.py (commenting out an IBM backend related check) that might be a holdover from some testing you were doing. Is that change intentional?

Great work on adding detailed docstrings throughout the new code. There are a couple of new methods that were added to the SummaryStatistics class in vb.py, avg_energy and outcome_energy, which are part of the public facing API but are currently missing docstrings, so adding those in would be useful. I also noticed that while most of the new functions added in randomcircuit.py as part of the private-facing API (i.e. with leading underscores) had comments associated with them describing their inputs, outputs and purposes, a couple of these functions didn't. While this is in my opinion more of a 'nice to have' than 'need to have', future you and us will undoubtedly be grateful for their inclusion.

Elephant in the room now (I'll save @sserita from having to be the bad guy this time around), we need to add some unit testing for these new modules. This can be a bit confusing the first time around with needing to understand the syntax/modules used for testing, but once you've made some once it isn't actually so bad. I'd be happy to go through this process with you and help write these new tests.

@jordanh6
Copy link
Contributor Author

jordanh6 commented Sep 8, 2023

Thanks! Also, thanks to @dhothem for writing a large portion of the code and documentation.

I think I resolved everything except writing unit tests. I'll happily take you up on your offer to help @coreyostrove !

  • Megred develop into feature-birb
  • The commented out IBMQ bit was leftover from debugging, so that's been undone
  • Added more docstrings.

jordanh6 and others added 4 commits September 8, 2023 10:23
This commit adds new unit tests targeting the construction of BiRB experiment designs as well as running the protocol and some basic checks for correct output in the ideal case. This commit also plugs holes in the previously existing set of unit tests for the remaining RB protocols (CRB, DRB, MRB) and adds tests for running those protocols (previously only the experiment design creation was being tested for these protocols). Finally, a  new test was added for the q_elimination sampling method for RB circuit construction.
@coreyostrove
Copy link
Contributor

With the new unit tests added (including some bonus ones extending our coverage for other parts of the RB codebase) and the last couple changes to the code addressing some edge cases from @jordanh6 this all looks good to me.

Copy link
Contributor

@sserita sserita left a comment

Choose a reason for hiding this comment

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

Thank you both for your work on this, I think it looks great! Really appreciate the unit tests and tutorial updates! :)

@sserita sserita merged commit 0f0f3cc into develop Sep 13, 2023
13 checks passed
@sserita sserita deleted the feature-birb branch September 13, 2023 16:25
@sserita sserita mentioned this pull request Oct 18, 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.

4 participants