Add discards_b8_out_path support to sinter file decoders - #1101
Conversation
Support reporting low-confidence discards through the file-decoder API, matching the existing trailing-byte discard semantics of compiled decoders. Decoders that accept the new optional discards_b8_out_path parameter write one byte per shot; sinter counts nonzero bytes as discards when computing conservative logical failure rates. DiskDecoder translates the file-based discards into a trailing prediction byte so compiled-path consumers handle them uniformly. Old decoders without the new parameter are detected via signature inspection and remain fully compatible.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
…s_b8_out_path support detection
|
@google-cla — the Individual CLA has now been signed (GitHub account linked). Could you re-run the check? Thanks! |
|
I have signed the Google Individual CLA and linked my GitHub account. Could the CLA check be re-run? Thanks! |
Done. Thank you for your contribution. |
There was a problem hiding this comment.
I don't think I want to merge this.
There are three problems here: breaking compatibility, complexity ratchet, and performance pitfall.
The breaking compatibility issue is that there are several existing decoders that provide a sinter.Decoder implementation, and you have added an argument that they need to implement to "properly" be a sinter.Decoder. I realize it's an optional argument and that if it's not specified things might happen to work, but still.
The complexity ratchet issue is that there are many, many things like 'record discards' that people might want to add to a decoder. For example, 'record errors' and 'record corrections' and 'record gaps' and 'record swim distances' and 'record runtime distribution' and 'record details of failed shots' and on and on and on. As we added those things, the notion of a decoder becomes more and more complicated. Eventually it becomes so complicated that the concept becomes unhelpful. I don't want to crank the rachet, and this change cranks the ratchet. (Avoiding the ratchet was the motivation for adding sinter.Sampler, which has a simpler less opinionated interface and for which it is far easier for users to hide complicated behavior behind.)
The performance pitfall is that the file writing methods were a mistake from the beginning, because when running many instances on many cores they end up losing parallelism due to contention for disk. It's better to keep things entirely in memory. (This was the motivation for adding the non-file-writing variants (inferred automatically from the file writing one if needed, to avoid breaking backwards compatibility).)
3 participants
Context
#297 identified that Sinter integrations drop the decoder low-confidence status, underestimating the conservative logical failure rate. The compiled-decoder path already signals discards via an extra trailing byte (classify_discards_and_errors in _stim_then_decode_sampler.py), but the file-decoder path (decode_via_files) has no channel for discards.
This PR extends the decode_via_files protocol with an optional discards_b8_out_path parameter (1 byte per shot, nonzero = discard), matching the established trailing-byte semantics. Decoders that don't support the new parameter are detected via inspect.signature and keep the old behavior - zero breakage.
Changes
Related