Swap register order, removing need to pass num_qpd_bits#434
Conversation
Pull Request Test Coverage Report for Build 6463654179
💛 - Coveralls |
caleb-johnson
left a comment
caleb-johnson
left a comment
There was a problem hiding this comment.
Dropping a couple thoughts here. Will finish this up in a bit. Looking great!
| if reg.name == "observable_measurements": | ||
| obs_creg = reg | ||
| break | ||
| else: |
There was a problem hiding this comment.
Neat code here where the else sits outside the looped if ... I'll have to glance at how this works :)
| for k, cog in enumerate(so.groups): | ||
| quasi_probs = results_dict[label].quasi_dists[i * len(so.groups) + k] | ||
| for outcome, quasi_prob in quasi_probs.items(): | ||
| try: |
caleb-johnson
left a comment
caleb-johnson
left a comment
There was a problem hiding this comment.
Great idea, thanks for puttin this PR together. Makes thing much more straightforward :)
| this vector correspond to the elements of ``cog.commuting_observables``, | ||
| and each result will be either +1 or -1. | ||
| """ | ||
| num_meas_bits = len(cog.pauli_indices) |
There was a problem hiding this comment.
I realized in my final-review-before-merging that this line has a bug. It should really be
| num_meas_bits = len(cog.pauli_indices) | |
| num_meas_bits = len(_get_pauli_indices(cog)) |
i.e., it needs to consider the actual number of measurement bits given the workaround to #422.
I fixed this and added a test (really, a more thorough version of the existing test_sampler_with_identity_subobservable) in f4dec60. The refactored test
- only considers a single circuit rather than all circuits given by the
example_circuitfixture (should save execution time) - performs a smoke test with
Sampler(as before) as well as a newly added full roundtrip test (which ensures correct expectation values) usingExactSampler
|
@ibrahim-shehzad: since @caleb-johnson already approved everything before f4dec60, and f4dec60 primarily updates a test that you wrote, I think it makes most sense for you to review the changes in f4dec60 and approve this PR if those changes look good to you. Thanks! |
ibrahim-shehzad
left a comment
ibrahim-shehzad
left a comment
There was a problem hiding this comment.
This looks good to me!
3 participants
This swaps the order of the registers such that
qpd_measurementsnow comes last. The motivation here is two-fold:num_qpd_bits