Leakage modeling: bugfixes for wildcard error and gauge optimization #671
Conversation
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
…obust to different labelings of the idenity gate, and make the returned model`s default_gauge_group have a direct sum structure; make the default leakage gaugeopt suite 2-stage, where the second stage is over the default gauge group in the target model)
rileyjmurray
requested review from
coreyostrove and
sserita
and removed request for
a team
rileyjmurray
commented
Oct 18, 2025
…Have lagoified_gopparams_dicts apply the direct sum group by default, since apparently the target model stored in list-of-dicts representation of `stdgaugeopt` suite overrides my attempt at setting the default gauge group
…pecific gate causes leakage
…orrectness test for leakage GST.
Contributor
Author
|
@coreyostrove I believe this is ready for review. When this guy is merged I can get to work on finalizing #669, which targets develop. Once that guy is merged I'll do the expansion of general leakage modeling functionality. That PR will include an overhaul of some functions in gaugeopt.py to avoid awkward use of ExplicitOpModelCalc. |
coreyostrove
requested changes
Oct 28, 2025
Contributor
coreyostrove
left a comment
coreyostrove
left a comment
There was a problem hiding this comment.
Thanks for all of your hard work, @rileyjmurray! I've left a few comments and questions for you which should hopefully be fairly quick to address.
| if transform is not None: | ||
| self_mx = self_mx @ transform | ||
| if isinstance(inv_transform, _mt.IdentityOperator): | ||
| other_mx = other_mx @ transform |
Contributor
There was a problem hiding this comment.
I don't understand lines 423 and 424, what do these do?
Contributor
Author
There was a problem hiding this comment.
Tentatively resolved by refactor in this commit.
pygsti/modelmembers/povms/effect.py
Outdated
| vec = transform.T @ vec | ||
| return _ot.frobeniusdist_squared(vec, other_spam_vec.to_dense()) | ||
| if isinstance(inv_transform, _mt.IdentityOperator): | ||
| other_vec = transform.T @ other_vec |
Contributor
Author
There was a problem hiding this comment.
Tentatively resolved by refactor in this commit.
Contributor
|
Aye
…On Tue, Oct 28, 2025, 8:41 AM Riley Murray ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pygsti/objectivefns/objectivefns.py
<#671 (comment)>:
> @@ -29,6 +31,11 @@
from pygsti.models.model import OpModel as _OpModel
from pygsti import SpaceT
+
+DEFAULT_MIN_PROB_CLIP = 1e-4
+DEFAULT_RADIUS = 1e-4
I have a small preference for defining these at the module level. The
inheritance structure of classes in objectivefns.py is already very
complicated, and I'd like to avoid adding complexity to the base classes.
That sound okay?
—
Reply to this email directly, view it on GitHub
<#671 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ASY6RPUNJTU6PGXKLYUBJDD3Z56CJAVCNFSM6AAAAACJQWPQVWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGOBZGEYTEMJVGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
…ics but no default implementation. I`ve added implementations for State, POVMEffect, and LinearOperator base classes. Using the new semantics of ModelMember._to_transformed_dense I was able to implement ModelMember.residuals in a way that should work for *all* child classes in a way that preserves old behavior. Using the new ModelMember.residuals definition I was able to add implementations of ModelMember.frobeniusdist and ModelMember.frobeniusdist_squared; these functions obviated the need for child classes to have their own implementations.
… account for leakage modeling. Remove wildcard1d_reference instance member from GSTBadFitOptions, and make it a read-only property always equal to "diamond distance".
rileyjmurray
commented
Oct 29, 2025
Contributor
Author
rileyjmurray
left a comment
rileyjmurray
left a comment
There was a problem hiding this comment.
@coreyostrove this is ready for another round of review
coreyostrove
approved these changes
Oct 29, 2025
Contributor
coreyostrove
left a comment
coreyostrove
left a comment
There was a problem hiding this comment.
Thanks for making the requested changes! Nothing else on my end, this all looks good. Merge at will.
Merged
rileyjmurray
added a commit
that referenced
this pull request
Oct 29, 2025
The auto-merge of #671 from bugfix into develop failed, so this PR is handling that manually. Description from bugfix merge below. > This PR is split off from #664. It fixes how wildcard error is computed in leakage modeling. When merged it will resolve #652. It also includes changes to the default leakage-aware gauge optimization suite. The latter changes are needed to prevent gauge optimization from trying to spuriously "spread out" relational errors across available gates. > > This PR adds a meaningful correctness test for leakage GST modeling. To reduce the time of the test I used a huge number of shots per circuit in the simulated dataset. This led to errors with "min_prob_clip" constants in objective function evaluation. In order to resolve those errors I replaced hard-coded instances of that constant's default value (1e-4) to values of a module-wide constant; I made similar changes for "radius" constants in objective functions. > > Finally, this PR adds implementations of frobeniusdist, frobeniusdist_squared, and residual to ModelMember, and removes the implementation of these functions from all child classes. Removing the child class implementation become possible because the ModelMember.residual implementation relies on a new method, ModelMember._to_transformed_dense(...), that child classes must implement. The semantics of _to_transformed_dense are documented in ModelMember. I provided implementations for base classes for POVM effects, states, and linear operators. > > (This PR is a superset of the since-closed PR #670, which started from develop instead of bugfix.)
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 PR is split off from #664. It fixes how wildcard error is computed in leakage modeling. When merged it will resolve #652. It also includes changes to the default leakage-aware gauge optimization suite. The latter changes are needed to prevent gauge optimization from trying to spurriously "spread out" relational errors across available gates.
This PR adds a meaningful correctness test for leakage GST modeling. To reduce the time of the test I used a huge number of shots per circuit in the simulated dataset. This led to errors with "min_prob_clip" constants in objective function evaluation. In order to resolve those errors I replaced hard-coded instances of that constant's default value (1e-4) to values of a module-wide constant; I made similar changes for "radius" constants in objective functions.
(This PR is a superset of the since-closed PR #670, which started from develop instead of bugfix.)