-
Notifications
You must be signed in to change notification settings - Fork 173
Add correlated_pymatching decoder to sinter builtin decoder list
#985
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
Conversation
correlated_pymatching decoder to sinter builtin decoder listcorrelated_pymatching decoder to sinter builtin decoder list
| def test_post_selection(decoder: str, force_streaming: Optional[bool]): | ||
| circuit = stim.Circuit(""" | ||
| X_ERROR(0.6) 0 | ||
| X_ERROR(0.4) 0 |
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.
This was potentially intentionally testing the ability to handle probabilities larger than 0.5. Why was it changed?
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.
If the correlated version of pymatching can't support errors larger than 50% yet, then I don't think it's ready to be added as a default supported decoder yet.
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.
Fair enough, but is it not still useful to include even if it doesn't support this? Surely p>0.5 errors are a much rarer property of practical stim circuits than matchability? I can look into supporting negative edge weights with correlations but it's possible it will require a major refactor. The approach I took in #1012 was to skip this test for pymatching-correlated.
| assert 650 <= result.discards <= 950 | ||
| if 'vacuous' not in decoder: | ||
| assert 40 <= result.errors <= 160 | ||
| assert 60 <= result.errors <= 240 |
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.
Why was this changed?
| custom_decoders=TEST_CUSTOM_DECODERS, | ||
| ) | ||
| assert 1050 <= result.discards <= 1350 | ||
| assert 650 <= result.discards <= 950 |
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.
Why was this changed to a non-overlapping range?
|
I just opened another similar PR (#1012) as I had forgotten there was already a WIP PR here. Since correlated pymatching doesn't support correlations I had it skip the test with the p>0.5 errors. |
|
Hi Oscar @oscarhiggott, I'll close this one in favor of your "official" implementation :-) |
Some tests failed due to issues in the correlated pymatching implementation. Wait for oscarhiggott/PyMatching#166 to be merged and a new patched version released. Then bump the
pymatchingversion requirement in decoder predicates added in this PR, e.g.:Update:
v2.3.1.test_post_selectionin_decoding_test.pya bit.