Improved symmetry verification API - #269
Conversation
antalszava
left a comment
There was a problem hiding this comment.
Nice change! 👏 Mostly nitpicky comments, a more important question is deprecation of symmetry_verification kwarg - can that break users' code?
Also, here are three points from Claude that I think are worth considering:
- Idempotency/caching: job.result() is normally expected to return the same thing each time. Adding postselect_on means repeated calls can differ, so be clear about whether raw results are cached once and filtered locally, and whether calling with no args still gives back the unfiltered result.
- Bit-order fragility: reachable_states needs to be defined against the classical register order the backend actually returns, not the pre-transpile order — otherwise transpilation could silently misalign the filter and discard the wrong shots with no error.
- Shot-count transparency: The filtered Result should clearly report both the discarded and retained shot counts, so downstream code doesn't mistake post-selected counts for the full original sample and get error bars/statistics wrong.
| ```python | ||
| print(job.result(aggregation="voting").get_counts()) | ||
| job = backend.run(qc, shots=1000) | ||
| result = job.result(postselect_on=job.reachable_states) |
There was a problem hiding this comment.
While I think things are well explained, this line now takes def more cognitive load for a user than the previous symmetry_verification=True did on backend.run.
Could symmetry_verification_state or something that refers to sv in its name work instead of reachable_states?
There was a problem hiding this comment.
In my experience post-selection is a pretty standard tool to apply, so that part I'm okay with. Reachable states definitely begs the question of how those are being computed and how is that different from actually simulating the circuit.
I'm in favor of keeping the naming symmetry verification in there somewhere, but I haven't found a way to include it in a way that's not forced. symmetry_verification_states feels more ambiguous.
There was a problem hiding this comment.
Got it! Not a blocker from my side - naming is hard, if someone else has inputs maybe that can help too, otherwise this point is not blocking for me
There was a problem hiding this comment.
Oh just realized: what's the significance of get_counts not having to be called anymore (e.g., in this example)? It feels like that with the new UI we do the post-selection, but simply that shouldn't implicitly mean that we also get the counts without having to call get_counts directly, right?
There was a problem hiding this comment.
Good catch, and you're right that you still need to call get_counts, but the examples in the README did not show that.
It did make me realize that we expose two ways to get counts:
job.result(...).get_counts()which is the way people should use aggregation/post-selectionjob.get_counts()which does not allow users to aggregate/post-select.
There's some maybe potentially confusing pieces about this beside the obvious one which is why are there two ways of doing the same thing? One example could be the difference in counts between the two methods:
job = ... # this is a made up example to illustrate the point
job.result(postselect_on={"00", "11"}).get_counts()
>>> {"00": 34, "11": 44}
job.get_counts()
>>> {"00": 34, "01": 3, "10": 8, "11": 44}IMO we should deprecate job.get_counts() since a user can always call job.result().get_counts() to get un-aggregated and pre-postselected results.
|
Thank you @antalszava for the very thorough review! Lots of great things pointed out. I'm still mulling over some of your bigger points, but for this one I need a bit more:
Any idea how we can help alleviate this? |
antalszava
left a comment
There was a problem hiding this comment.
Looks good! 🎉 Great one 🙂 👏
Shot-count transparency: The filtered Result should clearly report both the discarded and retained shot counts, so downstream code doesn't mistake post-selected counts for the full original sample and get error bars/statistics wrong.
Any idea how we can help alleviate this?
Good question - I'd guess it can be nice to provide as much info as we can on the discarded shot counts and otherwise just maybe have a good way of returning the number of discarded shots (or it can be left if no really good UI for it)
Summary
This PR moves the unreleased symmetry verification feature from a flag set on job creation, to an option when fetching results. This is achieved by adding a
reachable_statesproperty on theIonQJobobject which will now be computed for every job (instead of only when the users request symmetry verification).tldr
job = backend.run( circuit, shots=1000, - symmetry_verification=True, ) -result = job.result() +result = job.result( + postselect_on=job.reachable_states, +)This effectively gives users an api for doing post-selection as well if they want to pass arbitrary
Sequence[str]in forpostselect_on.Details and comments
Backend changes are needed for this to work. Not ready to merge until those land. (not linking here because private repo)