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 fidsel performance #280

Merged
merged 9 commits into from
Nov 28, 2022
Merged

Feature fidsel performance #280

merged 9 commits into from
Nov 28, 2022

Conversation

coreyostrove
Copy link
Contributor

This branch implements a new greedy search algorithm that leverages the same low-rank update tools now available for germ selection and FPR. This is specifically useful for accelerating fiducial selection for many-qubit systems in the 4+ qubit range (for 3 or less the existing algorithms seem to run quick enough for most applications). This new greedy search algorithm only works with an objective function based on the psuedoinverse-trace. Other notable changes in this branch:

  • Added additional flexibility to the specification of candidate fiducial sets. Before we could only specify a maximum fiducial length and then all circuits up to that length would be used in the search space. Now the API matches the one used for germ selection were we can specify a dictionary with keys corresponding to lengths and the ability to either provide special keywords like 'all upto' or a value in which case we sample a certain number of circuits at random. We still have compatibility with the old kwarg though.
  • There is a new deduping routine that can be used when the gate set members are generators of the clifford group (or some subset thereof). So gate sets like XYI or XYCPHASE etc... In this case a flag can be set which allows deduping the circuit list in amortized O(N) time.
  • Cleaned up a bunch of extraneous debug and print statements throughout various parts of the experiment design codebase (not just in fidsel)
  • Fleshed out incomplete germ selection docstring.

Corey Ostrove added 7 commits November 2, 2022 19:29
This commit introduced a new greedy search algorithm leveraging the same low-rank update tricks that we use in other parts of the experiment design codebase. This also introduces a new deduping routine specialized to the case where our gate set only generates the clifford group in which we leverage the fact that the PTMs of clifford operations are signed permutation matrices. This coupled with hash table magic (using python dictionaries which use them internally) gives an amortized O(N) deduping routine in this case.
Add additional flexibility to the candidate set specification by essentially adopting a slightly modified version of the code used for germ selection. This now allows for specifying a random set of circuits at a particular length instead of always including all up that length (which grows quickly combinatorically).
minor typo fix
restore backwards compatibility by allowing max_fid_length as an argument still, but mapping it's behavior to the equivalent one using candidate_fid_counts and raise a warning so the user knows what is going on.
Updates the docstring for germ selection to include explanations of some new options relevant to low-rank update based greedy search.
Removes a bunch of print statements that were included for debugging and also moves some of this information into appropriate logging functions/exception handling.
Fixes linting error from undefined variable.
@coreyostrove coreyostrove marked this pull request as ready for review November 21, 2022 03:44
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.

Haven't checked the math for the new greedy code path closely, given it's the same process as the recent FPR and germ sel low rank updates.
I think there is one place where code can be consolidated, i.e. we can dedup the dedup code.

pygsti/algorithms/fiducialselection.py Outdated Show resolved Hide resolved
pygsti/algorithms/fiducialselection.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@enielse enielse left a comment

Choose a reason for hiding this comment

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

Great stuff! I added a few suggestions that you can take or leave.

pygsti/algorithms/fiducialselection.py Show resolved Hide resolved
if final_test_fiducial_list:
printer.log('Final test of the candidate prep fiducial lists passed.', 1)
else:
printer.log('Final test of the candidate prep fiducial lists failed.', 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The above block is repeated 8 (I think) times within this function, which makes me think it'd be worth consolidating it to a nested function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refactored this as suggested.

pygsti/algorithms/fiducialselection.py Outdated Show resolved Hide resolved
pygsti/algorithms/germselection.py Outdated Show resolved Hide resolved
Refactors the deduping code to combine the two separate functions for deduping the candidate fiducial list into a single one that branches on the case where we are assuming a Clifford gate set. Also refactors a printing subroutine to reduce amount of repeating. Finally adds some docstring and debug clean-ups.
@coreyostrove
Copy link
Contributor Author

Thanks for the comments and feedback, @sserita and @enielse . I have refactored and updated the code in accordance with your suggestions. I'll plan on merging this in once I get a clean set of unit tests back on the latest commit (@sserita, I think you'll also need to approve the PR before merging is unblocked).

@sserita sserita merged commit 8998503 into develop Nov 28, 2022
@sserita sserita deleted the feature-fidsel-performance branch November 28, 2022 16:51
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.

3 participants