-
Notifications
You must be signed in to change notification settings - Fork 54
Change LGST Circuit Storage with FPR (Color Boxplot Fix) #561
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
Conversation
Change the way the LGST circuits are stored when using FPR. Previously these were stripped from the plaquettes and any needed circuits were added back in in _additional_circuits. Now we just don't remove those for L=1 if we shouldn't. This addresses a bug that was present in the color box plots where the result of the old behavior was that these circuits weren't rendered with the plaquettes.
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.
One point of discussion.
pygsti/circuits/gstcircuits.py
Outdated
@@ -528,7 +527,7 @@ def add_to_plaquettes(pkey_dict, plaquette_dict, base_circuit, maxlen, germ, pow | |||
if nest: | |||
#keep track of running quantities used to build circuit structures | |||
running_plaquette_keys = {} # base-circuit => (maxlength, germ) key for final plaquette dict | |||
running_plaquettes = _collections.OrderedDict() # keep consistent ordering in produced circuit list. | |||
running_plaquettes = dict() # keep consistent ordering in produced circuit list. |
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.
Just a general note that I've noticed several of these OrderedDict -> dict changes in recent PRs. It's not a problem, but just wanted to link it back to our discussion in #427 where we wanted to annotate dicts where order is explicitly important (e.g. where keys/values are iterated over with an expected order later).
I only bring this up here since "keeping consistent ordering" is part of the comment for this dict instantiation - is this one such place where order matters, and we should finalize our decision from #427?
Yup, good catch. I have been adding the annotations where relevant (which
has actually been pretty uncommon, it turns out), but forgot it seems. I'll
add these back in when I get a chance.
…On Mon, Mar 31, 2025 at 4:53 PM Stefan Seritan ***@***.***> wrote:
***@***.**** commented on this pull request.
One point of discussion.
------------------------------
In pygsti/circuits/gstcircuits.py
<#561 (comment)>:
> @@ -528,7 +527,7 @@ def add_to_plaquettes(pkey_dict, plaquette_dict, base_circuit, maxlen, germ, pow
if nest:
#keep track of running quantities used to build circuit structures
running_plaquette_keys = {} # base-circuit => (maxlength, germ) key for final plaquette dict
- running_plaquettes = _collections.OrderedDict() # keep consistent ordering in produced circuit list.
+ running_plaquettes = dict() # keep consistent ordering in produced circuit list.
Just a general note that I've noticed several of these OrderedDict -> dict
changes in recent PRs. It's not a problem, but just wanted to link it back
to our discussion in #427
<#427> where we wanted to
annotate dicts where order is explicitly important (e.g. where keys/values
are iterated over with an expected order later).
I only bring this up here since "keeping consistent ordering" is part of
the comment for this dict instantiation - is this one such place where
order matters, and we should finalize our decision from #427
<#427>?
—
Reply to this email directly, view it on GitHub
<#561 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ASY6RPRMCNOMHTRJINAOGYD2XHBODAVCNFSM6AAAAABZZC6EDKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMZQGY4TCNRRGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Add flags indicating the need for ordering information in a couple refactored formerly OrderedDicts.
This changes the way the LGST circuits are stored when using FPR. Previously these were stripped from the plaquettes and any needed circuits were added back in in
_additional_circuits
. Now we just don't remove those for L=1 if we shouldn't. This addresses a bug reported by @pcwysoc that was present in the color box plots where the result of the old behavior was that these LGST circuits weren't rendered with the plaquettes as expected.