Skip to content

Conversation

@enielse
Copy link
Collaborator

@enielse enielse commented Oct 7, 2025

Corrects an indexing bug that would cause DataSet lookups to silently fail (return incorrect values) when data had been added with dictionaries that didn't have keys ordered in the same way as the DataSet's outcome list. This commit fixes this issue and adds several unit tests used in debugging.

Previously failing minimal example (now included as a unit test) is:

import pygsti

c = "Gxpi2:0@(0)"
counts = {'00': 1, '10': 0, '01': 97, '11': 2}
ds = pygsti.data.DataSet(outcome_labels=["00", "10", "01", "11"], static=False)
ds.add_count_dict(c, counts)
check = ds[c].counts
print(ds)

assert {k[0]: v for k,v in check.items()} == counts

Corrects an indexing bug that would cause DataSet lookups to
silently fail (return incorrect values) when data had been added
with dictionaries that didn't have keys ordered in the same way as
the DataSet's outcome list.  This commit fixes this issue and adds
several unit tests used in debugging.
@enielse enielse requested review from a team and rileyjmurray as code owners October 7, 2025 19:55
@enielse enielse requested review from coreyostrove and removed request for a team October 7, 2025 19:55
@coreyostrove
Copy link
Contributor

Quick note for posterity that the unit tests for this PR are passing on the ionq repo (https://github.com/ionq/pyGSTi/actions/runs/18324481263). Quick clarifying question, the second unit test you added looks like it is testing against exactly the example give in issue #661. Am I correct that this PR also fixes that bug?

Copy link
Contributor

@coreyostrove coreyostrove left a comment

Choose a reason for hiding this comment

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

Looks good to me, approved! Thanks, as always, for this bugfix @enielse!

P.S. I posted a quick clarification question above.

@coreyostrove coreyostrove added this to the 0.9.14.2 milestone Oct 8, 2025
@coreyostrove coreyostrove merged commit bb89527 into sandialabs:bugfix Oct 8, 2025
@coreyostrove
Copy link
Contributor

Update: @enielse, just saw your comment on #661, I'll go ahead and mark that as fixed.

coreyostrove added a commit that referenced this pull request Oct 9, 2025
…evelop Merge) (#665)

Note: Since the automatic merge from bugfix into develop didn't work
this is just a manual merge of PR #663 into develop.

Co-authored-by: Erik Nielsen <nielsen@ionq.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants