Skip to content

fix: key detector coordinates by detector ID and bounds check array a… #272

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.

Sign up for GitHub

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

Open
aria-googler wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: key detector coordinates by detector ID and bounds check array a… #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a1143c
fix: key detector coordinates by detector ID and bounds check array a…
aria-googler Jul 26, 2026
f8cc040
Merge branch 'main' into fix/detector-coords-indexing
LalehB Aug 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/simplex.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ SimplexDecoder::SimplexDecoder(SimplexConfig _config) : config(_config) {

std::vector<double> detector_t_coords(config.dem.count_detectors(), 0);
std::vector<std::vector<double>> detector_coords = get_detector_coords(config.dem);
for (size_t d = 0; d < detector_coords.size(); ++d) {
size_t num_dets = std::min(detector_coords.size(), detector_t_coords.size());
for (size_t d = 0; d < num_dets; ++d) {
if (detector_coords[d].size() > T_COORD) {
detector_t_coords[d] = detector_coords[d][T_COORD];
}
Expand Down
16 changes: 16 additions & 0 deletions src/tesseract.test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,3 +559,19 @@ TEST(tesseract, MoreThan64Observables) {
ASSERT_EQ(flipped[i], i);
}
}

TEST(utils, DuplicateDetectorCoords) {

@LalehB LalehB Aug 8, 2026

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.

Can we add a regression test where D0 has no coordinates but D1 does? That’s the case that exposes the coordinate-ordering issue

std::string dem_str = "detector(0, 0, 1) D0\ndetector(0, 0, 2) D0\nerror(0.1) D0\n";
stim::DetectorErrorModel dem(dem_str.c_str());
auto coords = get_detector_coords(dem);
ASSERT_EQ(coords.size(), 1);
ASSERT_EQ(coords[0].size(), 3);
ASSERT_EQ(coords[0][2], 2.0);
}

TEST(simplex, DuplicateDetectorCoords) {
std::string dem_str = "detector(0, 0, 1) D0\ndetector(0, 0, 2) D0\nerror(0.1) D0\n";
stim::DetectorErrorModel dem(dem_str.c_str());
SimplexConfig config{dem};
EXPECT_NO_THROW({ SimplexDecoder decoder(config); });
}
27 changes: 20 additions & 7 deletions src/utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
#include "stim.h"

std::vector<std::vector<double>> get_detector_coords(const stim::DetectorErrorModel& dem) {
std::vector<std::vector<double>> detector_coords;
size_t num_detectors = dem.count_detectors();
std::vector<std::vector<double>> detector_coords(num_detectors);
bool has_any_coords = false;
for (const stim::DemInstruction& instruction : dem.flattened().instructions) {
switch (instruction.type) {
case stim::DemInstructionType::DEM_SHIFT_DETECTORS:
Expand All @@ -38,11 +40,16 @@ std::vector<std::vector<double>> get_detector_coords(const stim::DetectorErrorMo
break;
}
case stim::DemInstructionType::DEM_DETECTOR: {
std::vector<double> coord;
for (const double& t : instruction.arg_data) {
coord.push_back(t);
has_any_coords = true;
std::vector<double> coord(instruction.arg_data.begin(), instruction.arg_data.end());
for (const stim::DemTarget& target : instruction.target_data) {
if (target.is_relative_detector_id()) {
size_t det_id = target.val();
if (det_id < num_detectors) {
detector_coords[det_id] = coord;
}
}
}
detector_coords.push_back(coord);
break;
}
case stim::DemInstructionType::DEM_LOGICAL_OBSERVABLE:
Expand All @@ -52,6 +59,9 @@ std::vector<std::vector<double>> get_detector_coords(const stim::DetectorErrorMo
"Unexpected DemInstructionType found in the detector error model.");
}
}
if (!has_any_coords) {
return {};
}
return detector_coords;
}

Expand Down Expand Up @@ -148,10 +158,13 @@ static std::vector<std::vector<size_t>> build_det_orders_coordinate(
for (size_t i = 0; i < detector_coords.at(0).size(); ++i) {
orientation_vector.push_back(dist(rng));
}
for (size_t i = 0; i < detector_coords.size(); ++i) {
size_t num_dets = std::min(detector_coords.size(), inner_products.size());
for (size_t i = 0; i < num_dets; ++i) {
inner_products[i] = 0;
for (size_t j = 0; j < orientation_vector.size(); ++j) {
inner_products[i] += detector_coords[i][j] * orientation_vector[j];
if (j < detector_coords[i].size()) {

@LalehB LalehB Aug 8, 2026

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.

This still assumes D0 defines the coordinate dimensionality. If D0 has no coordinates but a later detector does, the check above returns identity orderings. If D0 has fewer dimensions, the extra dimensions on later detectors are ignored because orientation_vector is sized from detector_coords[0].
Could we determine the maximum coordinate length across all detectors, and only fall back to identity when every coordinate vector is empty?

inner_products[i] += detector_coords[i][j] * orientation_vector[j];
}
}
}
std::vector<size_t> perm(dem.count_detectors());
Expand Down
Loading