Skip to content

Commit 24ec0fd

Browse files
Address claude code review
1 parent a8cb50f commit 24ec0fd

5 files changed

Lines changed: 49 additions & 30 deletions

File tree

examples/skeleton/teasar.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
def main():
1212
path = Path(__file__).with_name("00004_gt_mask.mrc")
1313

14-
full_crop = False
14+
full_crop = True
1515
if full_crop:
1616
bb = np.s_[:]
1717
else:

include/bioimage_cpp/skeleton/detail/compact_grid_dijkstra.hxx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,28 @@ struct CompactGridDomain {
5252
[[nodiscard]] bool has_full_lookup() const noexcept {
5353
return !full_to_compact.empty();
5454
}
55+
56+
// CSR domains release the dense reverse map after building adjacency.
57+
// Keep full-index conversion independent of that storage decision.
58+
[[nodiscard]] std::uint32_t compact_node_from_full(
59+
const std::size_t full
60+
) const noexcept {
61+
if (has_full_lookup()) {
62+
return full < full_to_compact.size()
63+
? full_to_compact[full] : kNoCompactNode;
64+
}
65+
if (full > static_cast<std::size_t>(kNoCompactNode)) {
66+
return kNoCompactNode;
67+
}
68+
const auto full32 = static_cast<std::uint32_t>(full);
69+
const auto it = std::lower_bound(
70+
compact_to_full.begin(), compact_to_full.end(), full32
71+
);
72+
if (it == compact_to_full.end() || *it != full32) {
73+
return kNoCompactNode;
74+
}
75+
return static_cast<std::uint32_t>(it - compact_to_full.begin());
76+
}
5577
};
5678

5779
inline void build_compact_neighbors(

include/bioimage_cpp/skeleton/distributed/border_targets.hxx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,12 +277,12 @@ std::vector<LabeledVoxelTarget<LabelT>> border_targets_impl(
277277
}();
278278
const auto edge_distance = std::min({
279279
static_cast<long double>(spacing[face_axes[0]]) *
280-
(static_cast<long double>(run.y) - 0.5L),
280+
(static_cast<long double>(run.y) + 0.5L),
281281
static_cast<long double>(spacing[face_axes[0]]) *
282282
(static_cast<long double>(height) - 0.5L -
283283
static_cast<long double>(run.y)),
284284
static_cast<long double>(spacing[face_axes[1]]) *
285-
(static_cast<long double>(x) - 0.5L),
285+
(static_cast<long double>(x) + 0.5L),
286286
static_cast<long double>(spacing[face_axes[1]]) *
287287
(static_cast<long double>(width) - 0.5L -
288288
static_cast<long double>(x)),

include/bioimage_cpp/skeleton/teasar.hxx

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -625,26 +625,12 @@ inline LatticeSkeletonGraph teasar_compact_impl(
625625
{
626626
BIOIMAGE_PROFILE_SCOPE(profile, "root_dijkstra")
627627
if (required_root != std::numeric_limits<std::size_t>::max()) {
628-
if (required_root > std::numeric_limits<std::uint32_t>::max()) {
629-
throw std::runtime_error(
630-
"prepared required root exceeds compact index range"
631-
);
632-
}
633-
const auto it = std::lower_bound(
634-
domain.compact_to_full.begin(), domain.compact_to_full.end(),
635-
static_cast<std::uint32_t>(required_root)
636-
);
637-
if (
638-
it == domain.compact_to_full.end() ||
639-
*it != static_cast<std::uint32_t>(required_root)
640-
) {
628+
root = domain.compact_node_from_full(required_root);
629+
if (root == detail::kNoCompactNode) {
641630
throw std::runtime_error(
642631
"prepared required root is not foreground"
643632
);
644633
}
645-
root = static_cast<std::uint32_t>(
646-
std::distance(domain.compact_to_full.begin(), it)
647-
);
648634
} else {
649635
std::vector<Distance> first_field;
650636
detail::compact_physical_distance_field<Adjacency>(
@@ -808,19 +794,11 @@ inline LatticeSkeletonGraph teasar_compact_impl(
808794
if (full_target >= n) {
809795
throw std::runtime_error("prepared required target is out of bounds");
810796
}
811-
const auto it = std::lower_bound(
812-
domain.compact_to_full.begin(), domain.compact_to_full.end(),
813-
static_cast<std::uint32_t>(full_target)
814-
);
815-
if (
816-
it == domain.compact_to_full.end() ||
817-
*it != static_cast<std::uint32_t>(full_target)
818-
) {
797+
const auto target = domain.compact_node_from_full(full_target);
798+
if (target == detail::kNoCompactNode) {
819799
throw std::runtime_error("prepared required target is not foreground");
820800
}
821-
compact_required_targets.push_back(static_cast<std::uint32_t>(
822-
std::distance(domain.compact_to_full.begin(), it)
823-
));
801+
compact_required_targets.push_back(target);
824802
}
825803
std::sort(
826804
compact_required_targets.begin(), compact_required_targets.end(),

tests/skeleton/test_distributed.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,25 @@ def test_border_targets_binary_and_label_semantics_and_corner_deduplication():
6060
assert sum(len(values) for values in labeled.values()) == 2
6161

6262

63+
def test_border_target_edge_tiebreak_uses_physical_low_edge_distance():
64+
face = np.array(
65+
[
66+
[0, 0, 1, 0, 0, 0, 1, 0, 1, 0],
67+
[0, 1, 0, 0, 0, 1, 1, 1, 1, 0],
68+
[1, 0, 0, 1, 1, 1, 0, 0, 1, 0],
69+
],
70+
dtype=np.uint8,
71+
)
72+
block = np.zeros((3, 10, 2), dtype=np.uint8)
73+
block[:, :, -1] = face
74+
75+
targets = dist.block_border_targets(
76+
block, [(2, "high")], spacing=(2.0, 1.0, 1.0)
77+
)
78+
79+
np.testing.assert_array_equal(targets, [[1, 1, 1], [2, 4, 1]])
80+
81+
6382
def test_block_teasar_required_target_and_existing_teasar_equivalence():
6483
mask = np.zeros((9, 9, 13), dtype=np.uint8)
6584
mask[2:7, 2:7, 1:12] = 1

0 commit comments

Comments
 (0)