-
Notifications
You must be signed in to change notification settings - Fork 131
Enable averaging PUB params #1602
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
Enable averaging PUB params #1602
Conversation
| .metadata.get("circuit_metadata", {}) | ||
| .get("average_params", False) | ||
| ) | ||
| if outer_shape and not average_params: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you call join_data(), the BitArray for level 2 or ndarray for level 1 gets concatenated on the last axis. The next to last dimension is the shot index. When there are parameters, the earlier dimensions are the parameter indices. Allowing the data to pass this error for the BitArray case leads to get_counts() and get_bitstrings() getting called and that leads to averaging over parameters because those methods produce one combined counts dict and a one dimensional list of bitstrings (unraveling the parameter dimensions into the shots dimension effectively).
I have a few thoughts on this:
- Maybe we should not let level 1
ndarraycase get past the error here. I think the pub dimensions will make the arrays not match and lead to an exception on thedata["memory"]assignment lines below. - Maybe we should change
data["shots"]below to belen(data["memory"])? Otherwise, it will be the single parameter value shot number instead of the combined shot total. I think most of our analysis doeslen(bitstrings)orsum(counts.value())any way rather than relying on the separate shots number. - If you wanted, you could add a simple test with an outer parameter dimension and check that the
data()has the right shape. The risk is if we ever try to support parameterized pubs more fully we might break this option. Maybe that risk is pretty low.
| if ( | ||
| outer_shape | ||
| and not average_params | ||
| and not isinstance(joined_data, BitArray) | ||
| ): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the logic needs to be outer_shape and (not average_params or not isinstance(joined_data, BitArray)) or outer_shape and not (average_params and isinstance(joined_data, BitArray))?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right. thanks!
wshanks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
This enable when the circuit metadata contain
average_params.