Added bundled decoder_context struct for measurement extraction#671
Conversation
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
6cecabe to
224ee96
Compare
Signed-off-by: Eliot Heinrich <38039898+eliotheinrich@users.noreply.github.com> Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
224ee96 to
136e54c
Compare
There was a problem hiding this comment.
Thanks @eliotheinrich! This a great upgrade to the decoder interface. I have some comments regarding API and performance. Please see inline comments.
Probably out of scope for this PR given the timeframe, I think we can consider deprecating the legacy APIs.
f4c0988 to
fea2189
Compare
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
fea2189 to
88908c1
Compare
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
Signed-off-by: Eliot Heinrich <38039898+eliotheinrich@users.noreply.github.com>
bmhowe23
left a comment
There was a problem hiding this comment.
The PR introduces two related types whose names make the relationship harder to follow than it needs to be in my opinion:
- decoder_context - the result struct (dem + m2d + m2o), returned by component methods
- decoder_context_handle - the lazy factory returned by decoder_context_from_memory_circuit; call .x/z/full_component() to get a decoder_context
A few specific issues:
-
Python class is named DecoderContext, not DecoderContextHandle (py_code.cpp:534)
In Python, DecoderContext is what C++ calls decoder_context_handle - the lazy factory, not the final result. The C++ decoder_context struct has no Python binding (component methods return tuples). So Python users see DecoderContext as the object with .x/z/full_component() methods, which reads as "this is the context" when it's really "this is the thing that produces contexts." Either name the Python class DecoderContextHandle to match C++, or flip the naming so the more user-facing object (the handle) gets the simpler name. -
Stray term memory_circuit_handle in two test comments
test_qec_stim.cpp:854 and test_dem.py:684 both say "decoder_context_from_memory_circuit returns a memory_circuit_handle" - a type that doesn't exist in the API. Should say decoder_context_handle. -
Possible naming inversion to consider
If the intent is that users mostly interact with the factory object (decoder_context_from_memory_circuit(...)) and rarely hold a decoder_context directly, it might read more naturally to give the factory the simpler name (decoder_context) and add a suffix to the result struct (decoder_inputs, decoder_config, or canonicalized_decoder_context). As written, the prominent user-facing object has the longer name with _handle, while the plain data struct has the cleaner name.
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
b95b0d8 to
edf1b2c
Compare
|
Good points, thank you Ben; I went with calling the factory object the |
Description
In order to process enqueued syndromes, the decoder needs access to a measurement map so it knows how to form detectors. Right now, this comes from
generate_timelike_sparse_detector_matrix, which does not include boundary detectors.Rather than refactor this function (which would require it to be aware of the stabilizer structure of the code), I include the
m2dmap calculated bydem_from_kernel, and bundle it into a new structdecoder_context, and provide a new helper functiondecoder_context_from_memory_circuitwith a similar signature todem_from_memory_circuit, which it essentially supersedes. This new function returns the DEM as well as the measurement map needed by the decoder. Thedecoder_contextstruct also provides helper methods for extracting the X/Z components, alleviating the need for helper functions (in the style of*_dem_from_memory_circuit) as well as allowing a single circuit analysis to provide both the X and Z decoder contexts.The PR also applies some changes to the
real_time_completedocumentation examples to use this function.New tests:
test_dem.py::test_decoder_context_from_memory_circuit_requires_noise_model- asserts a throw on thedecoder_context_from_memory_circuitif no noise model is providedtest_dem.py::test_decoder_context_and_components- asserts thatdecoder_context_from_memory_circuitmatches output of superceded*_dem_from_memory_circuittest_dem.py::test_decoder_context_d_sparse_layout()- asserts that.d_sparse()ondecoder_contextflattens m2d map into correct -1-terminated sparse vectortest_dem.py::test_decoder_context_single_type_code_empty_component- check that empty component (i.e. x-component of a repetition code) is consistenttest_qec_stim.cpp::checkTealtimeDecodeFromMemoryCircuit- end-to-end check of realtime API using decoder_contexttest_qec_stim.cpp::checkDecoderContextAndComponents- Verify DEM output bydecoder_context_from_memory_circuitmatches superseded*_dem_from_memory_circuit, and that extractingx/zcomponents is idempotent and consistent.Runtime / performance impact
N/A
Self-review checklist
Please confirm each item before requesting review. Check
[x]or strikethrough and explain.
Before requesting review
Scope and size
(if so, an issue has been raised).
Tests
just when it is missing.
EXPECT_*/assertchecks areinsufficient for algorithmic correctness.
Documentation
tracked.
Code style
snake_casevscamelCase) forthe area being modified.
Dependencies
OSRB tickets filed.