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.

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

Bugfix Cirq Datasets and LGST Bug #420

Merged
merged 2 commits into from Apr 16, 2024
Merged

Bugfix Cirq Datasets and LGST Bug #420

merged 2 commits into from Apr 16, 2024

Conversation

coreyostrove
Copy link
Contributor

This PR covers two bugfixes. The first is in the method for adding cirq trials to pygsti datasets which did not behave as expected for 2+ qubits. The default behavior of the histogram method of the cirq Result object, which is used for gathering aggregated counts for each measurement results is to index these results using integers corresponding to the binary string for an outcome. In pygsti we more commonly use the binary strings themselves as the key. To address this we have added new options to add_cirq_trial_result which controls conversion back to a bit string before adding it to the count dictionary.

The second bugfix was to the LGST routine in order to patch in support for datasets with sparse formats wherein we only include outcomes with non-zero counts in the count dict for a circuit. Previously the implementation assumed that the data set was in a dense format with zero entries listed.

Corey Ostrove added 2 commits April 15, 2024 17:50
Add the (now default) option to convert a from the standard format for outcome labels in cirq (integers) to the standard representation used in pyGSTi (bit strings).
Bugfix for LGST to work with sparse data set formats where the count dicts only contain outcomes with non-zero counts.
@coreyostrove coreyostrove added this to the 0.9.12.2 milestone Apr 16, 2024
@coreyostrove coreyostrove self-assigned this Apr 16, 2024
@coreyostrove coreyostrove requested review from a team as code owners April 16, 2024 00:07
Copy link
Contributor

@sserita sserita left a comment

Choose a reason for hiding this comment

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

Excellent work @coreyostrove!

# when using a sparse data set format it might not be the case
# that all effect labels are present (only ones with non-zero counts are)
# so return 0 for the fraction in that case.
EVec[0, i] = dsRow_fractions.get((effectLabel,), 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect, exactly the bugfix I was thinking of!

convert_int_to_binary : bool, optional (defaut True)
By default the keys in the cirq Results object are the integers representing
the bitstrings of the measurements on a set of qubits, in big-endian convention.
If True this uses the cirq function `cirq.big_endian_int_to_bits` to convert back
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering if we were going to get bit by endian-ness... Nice job covering that edge case!

@sserita sserita merged commit e80ef3d into develop Apr 16, 2024
4 checks passed
@sserita sserita deleted the bugfix-cirq-datasets branch April 16, 2024 00:14
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.

None yet

2 participants