Skip to content

New DEM canonicalization tests#568

Merged
bmhowe23 merged 11 commits into
NVIDIA:mainfrom
kaiqiy-nv:kaiqiy/fix-DEM-canonicalisation-new
Jul 1, 2026
Merged

New DEM canonicalization tests#568
bmhowe23 merged 11 commits into
NVIDIA:mainfrom
kaiqiy-nv:kaiqiy/fix-DEM-canonicalisation-new

Conversation

@kaiqiy-nv

Copy link
Copy Markdown
Collaborator

This PR tries to fix [B] 6234203 .

It also strengthen the test cases to expose the bug and test further:

  • Specify merge_strategy='independent' in test_pymatching_decode_to_observable_surface_code_dem, since correct DEMs can contain parallel matching edges.
  • Use the external Stim’s DEM results to compare with the cudaq-qec's results.
  • Stim tests use pytest.importorskip("stim") so unsupported platforms get skipped.

Thank you for looking at this PR.

@kaiqiy-nv kaiqiy-nv requested a review from bmhowe23 May 28, 2026 06:39
@copy-pr-bot

copy-pr-bot Bot commented May 28, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Comment thread libs/qec/lib/detector_error_model.cpp Outdated
@kaiqiy-nv kaiqiy-nv force-pushed the kaiqiy/fix-DEM-canonicalisation-new branch 2 times, most recently from 62ca48e to c374d76 Compare June 1, 2026 07:15
@bmhowe23

bmhowe23 commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

/ok to test c374d76

@bmhowe23

bmhowe23 commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Thanks, @kaiqiy-nv. Could you please investigate the errors and warnings in

For the warning, could we please cap it at ~10 messages per function invocation so as to not spam the user's console with a short distance code?

@kaiqiy-nv

Copy link
Copy Markdown
Collaborator Author

Hi @bmhowe23 ,

To cap warnings at 10 messages per function invocation -- DONE in the new push

Cause of the CI failure above here:

  1. We updated canonicalize_for_rounds to keep same-H diff-O signatures, which gives parallel edges in matching-graph terms.
  2. z_dem_from_memory_circuit calls canonicalize_for_rounds, so dem.detector_error_matrix now contains those parallel-edge columns.
  3. The failed test case test_sliding_window_1 builds the full decoder first qec.get_decoder(decoder_name, dem.detector_error_matrix). For pymatching, plain construction uses the default merge_strategy = disallow without observables provided, so it rejects the parallel edges, giving ValueError:.

Potential need to update sliding_window.cpp:

Setting merge_strategy="independent" alone does not fix the test logic because it only stops the construction crash, not the assertion. Without O, PyMatching decodes in column space and the full vs. windowed decoders pick different representative columns, so exact-equality still fails.

We may need further updates in sliding_window.cpp (slice and forward the observables matrix O per window, and decode/commit in observable space) to enable a genuinely observable-aware sliding-window decode — i.e., a faithful full-vs-window equivalence for matching decoders.

Do we need to implement this update?
If so, we can do it in another PR, and for now, we only consider to update the test_sliding_window.py.

Quick fix in the local test cases:

We have changed the asserts of the pymatching cases to compare syndrome reproduction ((H @ e) % 2 == syndrome) instead of exact equality, because same-H diff-O columns are identical in H, so the exact column is not unique but every valid correction must still reproduce the syndrome.
single_error_lut cases are kept for strict equality.

We have also added one more test case test_pymatching_parallel_edges_use_observable_faults to make up for direct-PyMatching coverage to make sure that H-only PyMatching rejects parallel edges while the O + merge_strategy path decodes successfully.

@kaiqiy-nv kaiqiy-nv force-pushed the kaiqiy/fix-DEM-canonicalisation-new branch from b53bdce to d0ca533 Compare June 2, 2026 11:24
kaiqiy-nv added 6 commits June 5, 2026 01:53
Signed-off-by: Kaiqi Yan <kaiqiy@nvidia.com>
Signed-off-by: Kaiqi Yan <kaiqiy@nvidia.com>
Signed-off-by: Kaiqi Yan <kaiqiy@nvidia.com>
Signed-off-by: Kaiqi Yan <kaiqiy@nvidia.com>
Signed-off-by: Kaiqi Yan <kaiqiy@nvidia.com>
Signed-off-by: Kaiqi Yan <kaiqiy@nvidia.com>
@kaiqiy-nv kaiqiy-nv force-pushed the kaiqiy/fix-DEM-canonicalisation-new branch from d0ca533 to c522b94 Compare June 5, 2026 01:54
@bmhowe23

bmhowe23 commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 0e431bb

@bmhowe23

Copy link
Copy Markdown
Collaborator

Hi @bmhowe23 ,

To cap warnings at 10 messages per function invocation -- DONE in the new push

Cause of the CI failure above here:

  1. We updated canonicalize_for_rounds to keep same-H diff-O signatures, which gives parallel edges in matching-graph terms.
  2. z_dem_from_memory_circuit calls canonicalize_for_rounds, so dem.detector_error_matrix now contains those parallel-edge columns.
  3. The failed test case test_sliding_window_1 builds the full decoder first qec.get_decoder(decoder_name, dem.detector_error_matrix). For pymatching, plain construction uses the default merge_strategy = disallow without observables provided, so it rejects the parallel edges, giving ValueError:.

Potential need to update sliding_window.cpp:

Setting merge_strategy="independent" alone does not fix the test logic because it only stops the construction crash, not the assertion. Without O, PyMatching decodes in column space and the full vs. windowed decoders pick different representative columns, so exact-equality still fails.

We may need further updates in sliding_window.cpp (slice and forward the observables matrix O per window, and decode/commit in observable space) to enable a genuinely observable-aware sliding-window decode — i.e., a faithful full-vs-window equivalence for matching decoders.

Do we need to implement this update? If so, we can do it in another PR, and for now, we only consider to update the test_sliding_window.py.

Quick fix in the local test cases:

We have changed the asserts of the pymatching cases to compare syndrome reproduction ((H @ e) % 2 == syndrome) instead of exact equality, because same-H diff-O columns are identical in H, so the exact column is not unique but every valid correction must still reproduce the syndrome. single_error_lut cases are kept for strict equality.

We have also added one more test case test_pymatching_parallel_edges_use_observable_faults to make up for direct-PyMatching coverage to make sure that H-only PyMatching rejects parallel edges while the O + merge_strategy path decodes successfully.

@kaiqiy-nv I believe the underlying cause of the problem was in the original QEC circuit. In particular, our memory circuits terminate after the stabilizer rounds but do not include the boundary detectors synthesized from the final data qubit readout. This makes the circuits too sensitive to errors in the final stabilizer rounds. The true fix for this is in work, and it will incorporate use of CUDA-Q detectors (freshly added to CUDA-Q) in NVIDIA/cuda-quantum#4602.

Bottom line: I would like to incorporate your updates, but please allow a bit of time to sort out the multiple PRs that it will take to properly resolve this.

@bmhowe23

bmhowe23 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

@kaiqiy-nv - now that #610 has been merged, can you check which parts of this PR should remain?

Edit: note that some underlying issues still remain (namely, need to include detectors formed from the final data qubit measurements), and those will be fixed separately before the next release.

…nicalisation-new

Signed-off-by: Kaiqi Yan <kaiqiy@nvidia.com>

# Conflicts:
#	libs/qec/lib/detector_error_model.cpp
#	libs/qec/python/tests/test_dem.py
#	libs/qec/python/tests/test_sliding_window.py


PR NVIDIA#610 landed the canonicalize_for_rounds fix (observable-aware merging,
zero-syndrome handling, order-independent composition) plus C++ regression
tests, superseding the core change originally in this PR. Drop the now-
redundant source and test edits (taken from main on merge) and retain only
the supplementary coverage that NVIDIA#610 does not have:

- test_z_dem_from_memory_circuit_against_stim_oracle: cross-checks the
  synthesized Steane Z-DEM against an independent Stim DEM (signatures + rates).
- test_pymatching_decodes_stim_surface_code_dem: decodes a Stim rotated
  surface-code DEM through the PyMatching plugin and checks error reduction.
- test_pymatching_parallel_edges_use_observable_faults: guards that the H-only
  path rejects parallel edges while the observable-aware path merges them.

Stim is optional (pytest.importorskip), so Stim-less platforms skip cleanly.

Signed-off-by: Kaiqi Yan <kaiqiy@nvidia.com>
@kaiqiy-nv kaiqiy-nv force-pushed the kaiqiy/fix-DEM-canonicalisation-new branch from 3544b20 to 8812529 Compare June 23, 2026 07:23
@kaiqiy-nv

Copy link
Copy Markdown
Collaborator Author

@bmhowe23 Thank you for your fix in PR 610.
I have updated this PR, and now there are only some complementary tests: Two Stim-oracle-based cross-compare tests and one parallel-edge test.

@bmhowe23

Copy link
Copy Markdown
Collaborator

/ok to test 8812529

@bmhowe23

Copy link
Copy Markdown
Collaborator

/ok to test 8f3e188

@bmhowe23 bmhowe23 changed the title fix: To fix DEM canonicalisation New DEM canonicalization tests Jul 1, 2026
@bmhowe23

bmhowe23 commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 49a8f5b

@bmhowe23 bmhowe23 enabled auto-merge (squash) July 1, 2026 03:53
@bmhowe23 bmhowe23 merged commit 17dbd92 into NVIDIA:main Jul 1, 2026
20 checks passed
@kaiqiy-nv kaiqiy-nv deleted the kaiqiy/fix-DEM-canonicalisation-new branch July 1, 2026 08:26
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