Swap register order, removing need to pass num_qpd_bits
#434
Conversation
Pull Request Test Coverage Report for Build 6463654179
💛 - Coveralls |
| 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 :)
| @@ -111,21 +109,7 @@ def reconstruct_expectation_values( | |||
| 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: | |||
| @@ -156,15 +136,11 @@ def _process_outcome( | |||
| 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! |
This swaps the order of the registers such that
qpd_measurementsnow comes last. The motivation here is two-fold:num_qpd_bits