-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: result appending post-processing #713
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #713 +/- ##
==========================================
+ Coverage 92.81% 92.83% +0.01%
==========================================
Files 71 71
Lines 8159 8178 +19
==========================================
+ Hits 7573 7592 +19
Misses 586 586 ☔ View full report in Codecov by Sentry. |
tests/test_hresult.py
Outdated
"d": Counter({"11111": 10, "10": 1}), | ||
"e": Counter({"1": 1}), | ||
"lst": Counter({"101101": 1}), | ||
} |
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.
In this counter, are we losing information about the correlation between different tags across shots?
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.
yes, but it is still preserved in the full results object. The conversion to pytket preserves it for example.
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 see. I think those correlations would be important in most cases. No harm in offering this format, but I think we should also offer a counter-like result format that doesn't lose any information except for the shot ordering. I guess this would have to be something like Counter({(("c", "111"), ("d", "10")): 1, ...})
?
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.
yeah maybe that makes more sense as a default
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.
Hmm, although even that format isn't sufficient to capture the order of results within a shot (e.g. whether the "d" came between the first and second "c" or not), representing different control-flow paths. For that, we may need a format like Counter({("c", 1), ("c", 1), ("d", 1), ("c", 1), ("d", 0)) : 1, ...})
.
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.
that's too noisy to be useful in this context I think
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.
We may need to offer several different "views" into results to serve different use cases.
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.
yep, we already have register_counts
for serving the traditional "classical register" style use, this RFC is effectively view number 2
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.
Sounds good to me.
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.
representing different control-flow paths
I think Counter(shots.results)
where shots
is an instance of HShots
already gets you this
To enable e.g. all the elements of an array to be collated in to the same bitstrings, see tests Flattens lists so can be used once entire arrays can be printed too.
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 to me
To enable e.g. all the elements of an array to be collated in to the same bitstrings, see tests
Flattens lists so can be used once entire arrays can be printed too.
So some guppy that looks like for a given shot:
results in post-processed output, over 100 shots