-
Notifications
You must be signed in to change notification settings - Fork 132
Clifford Mirror RB experiment #1090
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
Open
coruscating
wants to merge
63
commits into
qiskit-community:main
Choose a base branch
from
coruscating:mirrorrb-rebased
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
63 commits
Select commit
Hold shift + click to select a range
921c185
add mirror RB experiment and analysis files
albertzhu01 9cd7074
edit appropriate mirrorRB-relevant files for PR
albertzhu01 2de778b
remove prints and add paper ref
paco-ri a28e6d2
update docstrings
albertzhu01 9d440e8
delete commented series code
albertzhu01 8d656b6
fix pygsti version
albertzhu01 b0432fd
remove outcomes setting
albertzhu01 e1de0e4
cleaned RB tutorial
paco-ri 859a94e
move coupling map outside loop, delete elements_without_inv and int_c…
albertzhu01 c1444ae
reformat files with black and lint
albertzhu01 6857a48
moved Clifford sampling in edgegrab algo
paco-ri 8445f4a
use full connectivity for coupling map if coupling map is not provided
albertzhu01 63f171c
changed AnalysisResult imports
paco-ri bbf111f
edited BitGenerator and SeedSequence imports
paco-ri 2308072
removed MirrorRBPyGSTi from library init file
paco-ri ebfa4d6
fix full connectivity map generation code
albertzhu01 de33876
remove MirrorRBPyGSTi from mirror_rb_experiment.py, rb tutorial, and …
albertzhu01 6fccca1
fix coupling map full connectivity generation and update aer imports …
albertzhu01 d2f8ff9
reformat imports (temporarily) to pass lint with merged code from ups…
albertzhu01 a7b7a70
merged main branch
coruscating 20b36db
added edge grab docstring
coruscating c8c8a7b
moved sampler to its own file
coruscating ea53a5d
updated edgegrab
coruscating 07b4b28
coupling map
coruscating 31f565e
updated release note
coruscating 41f9e81
partial refactoring
coruscating 1f58ffb
fix rebase errors
coruscating 961068d
update docs and remove edgegrab from utils
coruscating 7d79339
changed 1Q from integer to clifford
coruscating e82df72
merge main
coruscating e3c9b61
fixed circuit length
coruscating e66a58d
fix rng and force default transpile
coruscating 52ce46b
fix bugs to pass tests
coruscating f06dbc9
restored some RB changes and fixed tests
coruscating e69701d
updated manual
coruscating e6f8a07
replaced missing function
coruscating 7651798
rewrote circuit generation logic and sampler
coruscating 400a993
update 2q+ code
coruscating 97a746d
fixed wrong indexing for general coupling maps
coruscating b32d331
lint and improve manual
coruscating d589aa6
Merge remote-tracking branch 'upstream/main' into mirrorrb-rebased
coruscating c70e150
update manual
coruscating a6c8ff8
refactored sampling utils and added tests
coruscating 999bd6f
added distribution property to `MirrorRB`
coruscating 3407385
merged main
coruscating 3b0ae70
lint and fix docs
coruscating d318e51
fix doc links
coruscating 530e5b6
merge main
coruscating fb23e99
Update tests
coruscating 7224f59
fix nonlocal test
coruscating 2a0f0c2
Merge remote-tracking branch 'upstream/main' into mirrorrb-rebased
coruscating 09fe450
address review comments
coruscating 4951cb1
change sampler output to list of named tuple
coruscating 585c053
change gate distribution to named tuple
coruscating c209d1e
merge main
coruscating 3812049
refactored experiment options
coruscating 3070431
Merge remote-tracking branch 'upstream/main' into mirrorrb-rebased
coruscating d423e58
subclass`MirrorRBAnalysis` from `RBAnalysis`
coruscating b71d098
change y-axis option to `analyzed_quantity`
coruscating 0510478
changed to `CouplingMap` type and added validation
coruscating 1c98049
added data processor node for analysis
coruscating fb41d6d
change sampler to return a generator
coruscating 7710f7c
Address review comments and fix tests
coruscating File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -272,16 +272,19 @@ def __init__( | |
| """ | ||
| super().__init__(seed) | ||
| self._gate_distribution = gate_distribution | ||
| self._coupling_map = coupling_map | ||
| self.coupling_map = coupling_map | ||
|
|
||
| @property | ||
| def coupling_map(self): | ||
| def coupling_map(self) -> CouplingMap: | ||
| """The coupling map of the system to sample over.""" | ||
| return self._coupling_map | ||
|
|
||
| @coupling_map.setter | ||
| def coupling_map(self, coupling_map): | ||
| self._coupling_map = coupling_map | ||
| def coupling_map(self, coupling_map: Union[List[List[int]], CouplingMap]) -> None: | ||
| try: | ||
| self._coupling_map = CouplingMap(coupling_map) | ||
| except (ValueError, TypeError) as exc: | ||
| raise TypeError("Invalid coupling map provided.") from exc | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💯 |
||
|
|
||
| def __call__( | ||
| self, | ||
|
|
@@ -338,12 +341,12 @@ def __call__( | |
| ) | ||
| ] | ||
|
|
||
| if not isinstance(self.coupling_map, List) or isinstance(self.coupling_map, CouplingMap): | ||
| raise QiskitError("The coupling map must be set correctly before sampling.") | ||
|
|
||
| layer_list = [] | ||
|
|
||
| for _ in range(length): | ||
| all_edges = self.coupling_map[:] # make copy of coupling map from which we pop edges | ||
| all_edges = self.coupling_map.get_edges()[ | ||
| : | ||
| ] # make copy of coupling map from which we pop edges | ||
| selected_edges = [] | ||
| while all_edges: | ||
coruscating marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| rand_edge = all_edges.pop(self._rng.integers(len(all_edges))) | ||
|
|
||
This file contains hidden or 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like you override almost all protected methods of the
StandardRB(except for the transpile logic that manages pulse gates and gate counts). If you really need to override every method and you cannot find any possibility for partly reusing base class code, probably we should design better base class for RB experiments (or, maybe RB Metaclass?). Just curious. @itokoUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good discussion point. Ideally, I prefer
MirrorRBshould be a subclass ofBaseExperiment(notStandardRB). I don't want to have a base class for RB because I prefer delegation (helper functions) rather than inheritance (subclassing) in order to reuse implementation. So it would be better if we could introduce helper functions that have what is currently implemented in_transpiled_circuits()and_get_basis_gates, and use them to implement_transpiled_circuits()for StandardRB and MirrorRB. Anyway, such kind of refactoring should be done later (maybe in a follow-up PR before release).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any thoughts on this point @coruscating ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that
MirrorRBis different enough fromStandardRBthat it's not really worth subclassing directly. I think helper functions sound good. Let's do it in a follow-up PR as you suggested.