Skip to content

Support Stim DEM strings in get_decoder#571

Open
vedika-saravanan wants to merge 16 commits into
NVIDIA:mainfrom
vedika-saravanan:pr-stim-dem-decoder-init
Open

Support Stim DEM strings in get_decoder#571
vedika-saravanan wants to merge 16 commits into
NVIDIA:mainfrom
vedika-saravanan:pr-stim-dem-decoder-init

Conversation

@vedika-saravanan
Copy link
Copy Markdown
Collaborator

@vedika-saravanan vedika-saravanan commented May 28, 2026

Description

Adds unified decoder initialization from either a parity-check matrix or raw Stim detector error model text.

This supports both existing PCM-based decoders and DEM-native decoders needed by #546. PCM-based decoders can parse DEM text into H, O, and error_rate_vec defaults, while DEM-native decoders can consume the raw DEM text without going through a lossy matrix conversion.

API (cudaq/qec/decoder.h)

  • Adds decoder_init = std::variant<sparse_binary_matrix, std::string> as the decoder registry construction input.
  • Keeps existing PCM construction paths:
    • get_decoder(name, H, options)
    • decoder::get(name, H, options)
  • Adds DEM string construction through the same entry points:
    • get_decoder(name, dem_text, options)
    • decoder::get(name, dem_text, options)
  • Adds dem_from_stim_text(dem_text) to parse Stim DEM text into detector_error_model.
  • Adds helper routing for PCM-based decoders so DEM-derived O and error_rate_vec are supplied as defaults when not explicitly provided by the user.

The DEM-to-detector_error_model parser is lossy: it extracts detector flips, observable flips, and per-error probabilities, but drops detector coordinates and separator-encoded correlation structure. DEM-native decoders should consume the raw string alternative in decoder_init.

Python: cudaq_qec.get_decoder(...) now accepts Stim DEM strings. Python-registered decoders currently receive parsed H plus DEM-derived O / error_rate_vec defaults, not raw DEM text.

Dependency / build

QEC now provides its own Stim dependency via FetchContent when libstim is not already available, so standalone QEC builds do not depend on parent CUDA-Q interim build artifacts.

Tests

C++ gtests and Python tests cover:

  • DEM string construction through get_decoder(...)
  • PCM construction still working
  • user-provided options overriding DEM-derived defaults
  • DEM parse edge cases
  • DEMs without observables
  • stim::DemTarget category assumptions

Out of scope / follow-ups

Chromobius plugin integration itself; detector-coordinate storage on detector_error_model; optional PyMatching-specific improvements; user-facing Sphinx/RST docs.

Runtime / performance impact

N/A

Self-review checklist

Please confirm each item before requesting review. Check [x] or strike
through and explain.

Before requesting review

  • I reviewed my own full diff in GitHub or my editor.
  • PR is in Draft if it is not yet ready for review.
  • Temporary / debugging changes have been removed.
  • Local test logs reviewed; no unexplained warnings or errors.
  • CI logs reviewed; no unexplained warnings or errors.
  • Full CI has been run.

Scope and size

  • PR is under ~1000 lines, or an exception is justified in the description.
  • Refactoring-only changes are isolated in their own PR(s).
  • No existing tests were disabled or modified just to make this PR pass
    (if so, an issue has been raised).

Tests

  • New functionality has new tests.
  • Tests fail if the new functionality is broken (including crashes), not
    just when it is missing.
  • Negative tests added where exceptions are expected.
  • Truth data added where simple EXPECT_* / assert checks are
    insufficient for algorithmic correctness.
  • CI runtime impact considered; team notified if significant.

Documentation

  • Public-facing APIs have Doxygen docs.
  • User-visible behavior changes have public docs, or a follow-up is
    tracked.
  • User-facing docs for new features are in a separate PR held until
    release (the docs site publishes immediately on merge to the default
    branch, so feature docs must not land before the feature ships).

Code style

  • Naming follows the existing convention (snake_case vs camelCase) for
    the area being modified.

Dependencies

  • No new third-party dependencies, or the team has been notified and
    OSRB tickets filed.

@vedika-saravanan vedika-saravanan force-pushed the pr-stim-dem-decoder-init branch 4 times, most recently from e034551 to b3aa44a Compare June 2, 2026 15:44
@vedika-saravanan vedika-saravanan marked this pull request as ready for review June 2, 2026 16:02
@vedika-saravanan vedika-saravanan requested review from bmhowe23, justinlietz and melody-ren and removed request for bmhowe23, justinlietz and melody-ren June 2, 2026 16:02
@vedika-saravanan vedika-saravanan force-pushed the pr-stim-dem-decoder-init branch from b3aa44a to bccd851 Compare June 2, 2026 16:19
@vedika-saravanan vedika-saravanan marked this pull request as draft June 2, 2026 16:57
@vedika-saravanan vedika-saravanan marked this pull request as ready for review June 2, 2026 17:03
@vedika-saravanan vedika-saravanan marked this pull request as draft June 3, 2026 03:04
@vedika-saravanan vedika-saravanan force-pushed the pr-stim-dem-decoder-init branch from 5b57205 to 6603369 Compare June 3, 2026 03:14
@vedika-saravanan vedika-saravanan marked this pull request as ready for review June 3, 2026 03:36
Comment thread libs/qec/include/cudaq/qec/decoder.h
Comment thread libs/qec/lib/CMakeLists.txt Outdated
Comment thread libs/qec/include/cudaq/qec/decoder.h Outdated
@vedika-saravanan vedika-saravanan force-pushed the pr-stim-dem-decoder-init branch from 71c1666 to 1b2c61c Compare June 5, 2026 17:47
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
@vedika-saravanan vedika-saravanan force-pushed the pr-stim-dem-decoder-init branch from 268ba9a to 85e9d2b Compare June 5, 2026 18:07
@NVIDIA NVIDIA deleted a comment from copy-pr-bot Bot Jun 5, 2026
@vedika-saravanan vedika-saravanan marked this pull request as draft June 5, 2026 19:09
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
@vedika-saravanan vedika-saravanan force-pushed the pr-stim-dem-decoder-init branch from 00a9838 to fb6dcfc Compare June 8, 2026 16:04
@vedika-saravanan vedika-saravanan force-pushed the pr-stim-dem-decoder-init branch from d6be6ce to f428c85 Compare June 8, 2026 17:08
…vailable

Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
@vedika-saravanan vedika-saravanan force-pushed the pr-stim-dem-decoder-init branch from f428c85 to 1253540 Compare June 8, 2026 17:30
@vedika-saravanan vedika-saravanan marked this pull request as ready for review June 8, 2026 17:44
@vedika-saravanan vedika-saravanan changed the title Enable decoder construction from Stim DEM strings Support Stim DEM strings in get_decoder Jun 8, 2026
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
@vedika-saravanan vedika-saravanan force-pushed the pr-stim-dem-decoder-init branch from ccb933f to 05b5d5a Compare June 8, 2026 18:27
Signed-off-by: vedika-saravanan <vsaravanan@nvidia.com>
@vedika-saravanan vedika-saravanan requested a review from bmhowe23 June 8, 2026 19:42
Copy link
Copy Markdown
Collaborator

@bmhowe23 bmhowe23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good...thanks, @vedika-saravanan. Just a few follow-up questions below.

Comment on lines +77 to +81
target_link_libraries(${LIBRARY_NAME} PRIVATE libstim)
target_link_options(${LIBRARY_NAME} PRIVATE
$<$<OR:$<CXX_COMPILER_ID:GNU>,$<CXX_COMPILER_ID:Clang>>:-Wl,--exclude-libs,libstim.a>
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I downloaded this PR and built locally (using 92d3dc9), and I see the following, which could be problematic:

$ nm -D lib/libcudaq-qec.so | grep stim | c++filt
00000000000907d0 W stim::DetectorErrorModel::~DetectorErrorModel()
00000000000907d0 W stim::DetectorErrorModel::~DetectorErrorModel()

The above two items are the only problematic items, so it's possible that your solution here covers most of the issue, but I don't think we should be leaking those symbol out.

In order to prevent something like NVIDIA/cuda-quantum#3045, I think something like NVIDIA/cuda-quantum#3076 is required.

EXPECT_EQ(decoder2->get_syndrome_size(), new_syndrome_size);
}

TEST(DecoderRegistryTest, SingleParameterRegistryDirect) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking - does constructing a decoder w/o heterogenous map no longer supported? I don't think this is very common, so it's probably ok if we deprecate it; I just wanted to double check because if it still works then I'd want to keep this test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants