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.

Sign up for GitHub

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

Jump to bottom

Bugfix cloud noise model callable ops #485

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Bugfix cloud noise model callable ops #485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d8c4cb2
strip args before matching cloud noise ops
AidanGauthier Jul 31, 2024
0d84bbf
embed ops to map sslbls
AidanGauthier Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pygsti/models/cloudnoisemodel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@ def _layer_component_cloudnoises(self, model, complbl_list, cache):

else:
for complbl in complbl_list:
complbl = complbl.strip_args() if complbl.collect_args() else complbl
if complbl in cache:
ret.append(cache[complbl]) # caches['cloudnoise-layers'] would hold "simplified" instrument members
elif complbl in model.operation_blks['cloudnoise']:
Expand Down
2 changes: 1 addition & 1 deletion pygsti/models/modelnoise.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ def apply_errormap_stencil(self, stencil, evotype, state_space, target_labels=No
# `None` in list signals a non-present direction => skip these terms
sslbls_list = list(filter(lambda x: x is not None, sslbls_list))
for sslbls in sslbls_list:
op_to_embed = local_errormap if (sslbls is None or state_space.is_entire_space(sslbls)) \
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not immediately obvious what bug would have been caused by the is_entire_space call. I'm sure it's fine, but could you explain what this fixes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, these two fixes come from Aidan and were edge cases he ran into while doing some CloudNoiseModel simulations. I understand the other fix, but forget what necessitated this fix. @AidanGauthier do you remember why we needed to do this?
@coreyostrove btw, I'll probably move this to a draft while updating the issue description. I want to fix #441 at the same time.

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 talked to Aidan and here's the edge case this caused: local_errorgen comes from the stencil, so it has generic stencil state space labels. However, the is_entire_space call only checks after the sslbls have been mapped on line 481. In the case the mapped state space fully matches, then the still-stencil errorgen is passed through, causing downstream sslbl mismatch errors.

EmbeddedOps presumably fix this because it is already doing sslbl mapping, but it's not the most efficient thing to do in this case. @coreyostrove, unless you have another suggestion, I'll probably expand this line into an if/elif/else and handle the is_entire_space in its own block where we copy the op with the correct sslbls instead of relying on EmbeddedOp to do so.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds perfect, thanks to you and @AidanGauthier for tracking this down and explaining!

op_to_embed = local_errormap if (sslbls is None) \
else _op.EmbeddedOp(state_space, sslbls, local_errormap)
embedded_errmaps.append(op_to_embed.copy() if copy else op_to_embed)

Expand Down