-
Notifications
You must be signed in to change notification settings - Fork 55
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
Add Eriks updates: experimental state-space-setting framework; lots of little fixes #375
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
In _compute_1d_reference_values_and_name, an alternate control path didn't check for cvxpy failure and fallback to the trace distance (after generating a warning). Now it does.
…ions. I don't know why these directories weren't needed before and seem to be needed on Erik's machine now, but I don't think they'll hurt anything being there and maybe they'll fix builds on other systems too.
…g is the Circuit's initial string representation.
…uits(...) This bug would cause Circuits to be constructed by convert_strings_to_circuits contrary to the value in Circuit.default_expand_subcircuits. Since the default is to expand subcircuits, this bug could cause the appearance of unexpected CircuitLabel objects within loaded Circuits.
…am matrix plot. Adds a try/except to gauge.py that catchs a KeyError when creating the report GramMatrixPlot and just creates a BlankTable in its place. This error can occur when the experiment design doesn't have a complete set of LGST circuits, which include the ones used for the Gram matrix plot. In the future, a more robust solution for ignoring/removing errors during report generation should be employed.
Fixes an argument ordering bug (that only creates a problem when unmodeled error is computed) and adds plumbing to signal the GatesVsTargetTable when to display an "unmodeled error" column for each gate.
Adds a setter method for the `ModelMember.state_space` property which calls the new method `_update_submember_state_spaces`. This new method's job is to update as necessary the state space of any submembers and auxiliary data (i.e. other than the _state_space attribute itself) needed to change the state space in the object, such as re-creating representation objects. This commit adds implementation for composed and embedded ops, and dense ops. Other op types may need specific implementation, but these are the main two types needed for changing the state spaces of many-qubit models. For example, it may be appropriate for dense ops to throw an error if the state space changes in size since this change cannot readily be done. Also, the _update_submember_state_spaces function should probably use a memo, following the pattern of other functions that operate on webs of modelmembers, so that modelmembers don't get asked to update their state spaces many times and get stuck in an infinite update loop. All that said, the current implementation allows changes of state space in simple many-qubit models.
…eration. This causes find_germs_breadthfirst germ selection to exit without adding any germs if the "forced" set of germs is sufficient to amplify all the parameters, which seems like it should be the expected behavior.
Tests that both LHS and RHS of test are lists/tuples before testing them for equality, which results in more informative errors being generated when they're not equal.
…pletely. Previously, in GST circuit construction when a maximum length was greater than the per-germ maximum length given by the `germ_length_limit` argument, the circuits for that germ were always omitted. This could be annoying, however, if *only* large maximum lengths are given and a germ has a max-length limit smaller than all the maximum lengths requested. This commit alters the behavior of the GST circuit construction routines to include the circuits for such a germ that have the germ's repeated length equals its max-length-limit (even though this limit is smaller than the first maximum length requested).
…when running GST. Adds logic that treats a gauge optimization suite object with `None` as its suite names (checks of `gaugeopt_suite.gaugeopt_suite_names is None`) as specifying that no gauge optimization should be performed. Such an object is build if gaugeopt_suite=None is passed when creating a GST protocol, and so we should handle this case gracefully (previously this would result in an error).
In use cases where lots of Hessian projections are performed this method can get send a lot to stdout and there was no way to shut it up. This commit allows the user to specify a verbosity when projecting a Hessian.
Thanks for the quick PR @enielse! |
sserita
approved these changes
Nov 28, 2023
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.
All looks good, thanks for all these fixes! There was no merge conflict with Corey's trivial gauge changes, so I will approve this and we can dedup the functionality as part of spring cleaning.
Hi Bas,
No worries, this branch will be staying up for now. I’ll urge to migrate over to pygsti 0.9.12 when it makes sense as it includes these bugfixes and many more, but I realize that is a process for many of our users.
I’ll also mention that at some point this branch will likely just be active in a fork of pyGSTi, but exactly what that looks like is TBD in the near future as we reorganize how we manage the repository.
…--
Stefan K. Seritan
From: Bas Nijholt ***@***.***>
Date: Wednesday, December 13, 2023 at 10:30 AM
To: sandialabs/pyGSTi ***@***.***>
Cc: Seritan, Stefan Kelley ***@***.***>, State change ***@***.***>
Subject: [EXTERNAL] Re: [sandialabs/pyGSTi] Add Eriks updates: experimental state-space-setting framework; lots of little fixes (PR #375)
Would it be possible to leave to branch up for a little while? We've internally pinned some stuff to this branch and not all of that broke 😭
—
Reply to this email directly, view it on GitHub<#375 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ARIOHDTPK3KPOJLQSVUFFUTYJHX4VAVCNFSM6AAAAAA7VAIKGOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJUGUYDCNJVGY>.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
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.
A number of accumulated minor fixes along with one experimental feature to allow setting a model member's state space labels. This updates
modelmember.py
,composedop.py
, andembeddedop.py
and is an experimental feature that hasn't been fully tested yet. In particular, caches of representation objects will need to be cleared in the forward simulator and/or model to get the approach to work robustly.Aside from this, the main fixes are:
verbosity
argument toConfidenceRegionFactory.project_hessian
gaugeopt_suite=None
to indicate no gauge optimization should be performed when constructing GST and model test protocol objects.setup.py
when building cython extensions. I think these are needed to build on some platforms, maybe M1 macs (?).