-
Notifications
You must be signed in to change notification settings - Fork 54
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 parity benchmarking #532
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a new sub-package: pygsti.extras.paritybenchmarking, that contains disturbancecalc.py - a module for computing the weight-k disturbance metric. A notebook demonstrating how to use the contained functions is given in the new jupyter_notebooks/Tutorials/other/ParityBenchmarking.ipynb notebook. Some cosmetic cleanup has been done to disturbancecalc.py from the initial version that was created for separate distribution - linting errors/warnings have been fixed, and imports have been underscore-prefixed.
- guards cvxpy import in distrubancecalc.py, as cvxpy is not a required package. - adds pygsti.extras.paritybenchmarking to setup.py's list, so that non-local installs work properly.
…tleneck. The swell function is used to construct basis sets for a disturbance calculation. The initial implementation was straightforward but relied on lots of numpy krons, etc. This commit adds a new version that makes use of pyGSTi's embedding logic to more efficiently embed a weight-k transition matrix within a larger one, and should speed up disturbance calculations significanty. This commit leaves in a bunch of debug logic and print statements, including a check within the swell function itself to compare its output with the old version - so right now the new version isn't any faster. Once we're satisfied the new implementation is correct we'll remove the debug apparatus.
… warnings. This was creating problems throughout the pygsti unit tests, where warnings are ok and should't raise exceptions.
Checks have completed and the new, fast version of `swell` seems correct. This commit removes the debugging apparatus used to check the performance of partity benchmarking on 6 qubits. It also adds output giving the time elapsed per bootstrap run, which for 6Q is ~1min per run on Erik's old laptop.
OVD calculation appears to be non-convex though, so we'll need to do something else. This commit partially plumbs a "reference" argument (for the ideal probability distribution P0), and experiments with some code - marked with "HERE" - in the appropriate spot within disturbancecalc.py, even though it doesn't work yet.
After much discussion, we decided to compute the Residual OVD by simply using the 'T' matrix found from a Residual TVD calculation in the formula for original variational distance (OVD).
…TVD point. We settled on computing the residual-OVD by evaluating it at the best "T" (transition) matrix for the residual-TVD (since finding the latter is a convex program). This additional local optimization tries to improve the T-matrix locally to achieve a better (lower) OVD. A warning is printed if this local optimization improves the OVD too much, since this may indicate that the initial point was bad and the final result isn't a global optimum.
When self.weight == 0 (for computing the weight-0 residual-OVD) the code still computed the TVD! Fixed now, and preliminary tests of disturbance using residual-OVD look as expected.
After some further testing, we have realized that the OVD doesn't seem compatible with the foliation we used to move from TVD -> ResidualTVD. In particular we find that the residual-OVD values, when computed similarly to residual-TVD, can be weirdly behaved as a function of spam error. This is at least in part due to the fact that the "T" (transfer mx) found by the residual-TVD computation may exactly match the TP and Q distributions on the 0000 and 1111 bit strings, leaving the only remaining discrepancies on bit strings without support in P0. To fix this issue, this commit redefines the residual-OVD to just be a scaled version of the residual-TVD (scaled by the total OVD/TVD ratio, as the OVD is still a robust metric).
…ions. Removes OVD computation from within ResidualTVD class, since we currently just correct the residual TVD by an overall r=OVD/TVD scaling factor. Adds a function to compute this scaling factor as well as separate ...ovd_corrected... functions for computing disturbance quantities that are scaled by the OVD/TVD ratio.
Parity benchmarking update
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds parity benchmarking features to
pygsti.extras
and an associated tutorial injupyter_notebooks/Tutorials/other
, where parity benchmarking refers to computing various weight-X disturbance metrics between a reference and test data set. This is a fairly standalone feature, but we would like to merge it in to prevent this functionality from getting lost on a stale branch.WIP: There are cells in the notebook that we should turn into unit tests, but otherwise, the code seems fully functional. @enielse, this is probably a blast from the past, but you are probably the last person who touched this code in earnest. Do you remember if there are any other outstanding issues that would prevent a merge here?