From 4bb2c2de773a56cce8d1529917f848fac1613166 Mon Sep 17 00:00:00 2001 From: Siddhardha Nanda <99672439+SID-6921@users.noreply.github.com> Date: Tue, 10 Nov 2026 19:30:55 -0500 Subject: [PATCH 1/3] fix(metrics): SurfaceDistanceMetric ignored channel leaks inf into aggregate compute_average_surface_distance did not special-case the ignore_index channel the way compute_hausdorff_distance does. create_ignore_mask zeroes the ground-truth for that channel entirely, so get_edge_surface_distance sees an empty target and returns inf (not nan) whenever the prediction still has any positive voxels there. do_metric_reduction only excludes nan, so that inf silently poisons SurfaceDistanceMetric.aggregate(), while HausdorffDistanceMetric correctly reports nan (excluded) for the same input and ignore_index. Mirror HausdorffDistanceMetric's early-exit: when the channel being scored is the ignore_index, short-circuit to nan instead of calling get_edge_surface_distance on the masked-out target. Added a regression test comparing the ignored channel's buffer value and the aggregate against the expected nan/finite behavior; it fails on the previous code (inf) and passes with the fix. Signed-off-by: Siddhardha Nanda <99672439+SID-6921@users.noreply.github.com> --- monai/metrics/surface_distance.py | 9 ++++++++- tests/metrics/test_surface_distance.py | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/monai/metrics/surface_distance.py b/monai/metrics/surface_distance.py index 6b7fc9609d..6a578a479b 100644 --- a/monai/metrics/surface_distance.py +++ b/monai/metrics/surface_distance.py @@ -225,6 +225,14 @@ def compute_average_surface_distance( class_offset = 0 if include_background else 1 for b, c in np.ndindex(batch_size, n_class): + absolute_c = c + class_offset + if ignore_index is not None and absolute_c == ignore_index: + # ponytail: mirror compute_hausdorff_distance's early-exit for the ignored + # channel instead of letting get_edge_surface_distance run on a ground truth + # that create_ignore_mask has zeroed out (which produced `inf`, not `nan`, + # and was silently included by do_metric_reduction). + asd[b, c] = torch.tensor(float("nan"), device=asd.device) + continue yp = y_pred[b, c] yt = y[b, c] if per_component: @@ -267,7 +275,6 @@ def compute_average_surface_distance( ) asd[b, c] = torch.nanmean(torch.stack(component_scores)) if component_scores else 0.0 else: - absolute_c = c + class_offset warn_empty = ignore_index is None or absolute_c != ignore_index _, distances, _ = get_edge_surface_distance( yp, diff --git a/tests/metrics/test_surface_distance.py b/tests/metrics/test_surface_distance.py index ff6edb2577..9d26fde849 100644 --- a/tests/metrics/test_surface_distance.py +++ b/tests/metrics/test_surface_distance.py @@ -235,6 +235,28 @@ def test_cc_metrics(self, input_data, expected_value): result = sd_metric.aggregate(reduction="none") np.testing.assert_allclose(result.cpu().numpy(), expected_value, atol=1e-4) + def test_ignore_index_excludes_channel_like_hausdorff(self): + # Regression test: SurfaceDistanceMetric used to leave the ignored channel as `inf` + # (included by do_metric_reduction, poisoning the aggregate) instead of `nan` like + # HausdorffDistanceMetric, because compute_average_surface_distance was missing the + # early-exit that compute_hausdorff_distance has for `absolute_c == ignore_index`. + y = torch.zeros((2, 3, 16, 16)) + y_pred = torch.zeros((2, 3, 16, 16)) + y[:, 1, 4:8, 4:8] = 1 + y[:, 2, 8:12, 8:12] = 1 + y[:, 0] = 1 - y[:, 1:].sum(dim=1) + y_pred[:, 1, 5:9, 5:9] = 1 + y_pred[:, 2, 8:12, 8:12] = 1 + y_pred[:, 0] = 1 - y_pred[:, 1:].sum(dim=1) + + sd_metric = SurfaceDistanceMetric(include_background=True, ignore_index=1, reduction="mean") + sd_metric(y_pred, y) + + sd_buffer = sd_metric.get_buffer() + self.assertTrue(torch.isnan(sd_buffer[:, 1]).all(), f"ignored channel should be nan, got {sd_buffer[:, 1]}") + sd_result = sd_metric.aggregate() + self.assertTrue(torch.isfinite(sd_result).all(), f"aggregate should not be inf/nan, got {sd_result}") + def test_channel_dimensions(self): with self.assertRaises(ValueError): SurfaceDistanceMetric(per_component=True)(torch.ones([3, 3, 144, 144]), torch.ones([3, 3, 144, 144])) From f1d2c720d33fc17c520df72a9ecf90589516913a Mon Sep 17 00:00:00 2001 From: Siddhardha Nanda <99672439+SID-6921@users.noreply.github.com> Date: Tue, 10 Nov 2026 19:46:05 -0500 Subject: [PATCH 2/3] fix(metrics): don't nan out the only channel for single-channel ignore_index CodeRabbit flagged a review comment on the previous commit's early-exit: with include_background=True, ignore_index=0, and single-channel (label-encoded) binary input, `absolute_c == ignore_index` is true for the only channel (c=0), so the unconditional early-exit forced that channel to nan for every image. do_metric_reduction's all-nan convention then silently collapses the aggregate to 0, reporting a perfect score regardless of the actual prediction. For single-channel input, ignore_index is a value-based spatial mask (already applied via create_ignore_mask before this loop runs), not a request to drop an entire one-hot channel the way it is for multi-channel input. Guard the early-exit with n_class > 1 so it only fires when ignore_index genuinely names a separate class channel to exclude. Also documented, per the second review comment, that a matching channel's per-image result is nan in the multi-channel case. Added test_ignore_index_single_channel_does_not_blank_the_only_score, which fails (0.0 not greater than 0.0) against the unguarded code and passes with the guard. Signed-off-by: Siddhardha Nanda <99672439+SID-6921@users.noreply.github.com> --- monai/metrics/surface_distance.py | 14 ++++++++++---- tests/metrics/test_surface_distance.py | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/monai/metrics/surface_distance.py b/monai/metrics/surface_distance.py index 6a578a479b..05af5c7949 100644 --- a/monai/metrics/surface_distance.py +++ b/monai/metrics/surface_distance.py @@ -69,7 +69,8 @@ class SurfaceDistanceMetric(CumulativeIterationMetric): ignore_index: single integer class index (or sentinel value) to ignore from the metric computation. Voxels with this label are excluded from the score, which is useful for padding, unlabeled regions, or boundary artifacts. For federated or aggregated settings, ensure all clients use the same - ignore_index to keep score values comparable. + ignore_index to keep score values comparable. For multi-channel (one-hot) input, when + ``ignore_index`` names an existing channel that channel's per-image result is ``nan``. per_component: whether to compute the Surface Distance on a per-connected component basis. Defaults to ``False``. """ @@ -203,7 +204,8 @@ def compute_average_surface_distance( for all images in batch. Defaults to ``None``. ignore_index: optional class index used to suppress empty-mask warnings when that class is ignored. For federated or aggregated settings, ensure all clients use the same ignore_index to keep score - values comparable. + values comparable. For multi-channel (one-hot) input, when ``ignore_index`` names an existing + channel that channel's per-image result is ``nan``. per_component: whether to compute the Surface Distance on a per-connected component basis. Defaults to ``False``. """ @@ -226,11 +228,15 @@ def compute_average_surface_distance( for b, c in np.ndindex(batch_size, n_class): absolute_c = c + class_offset - if ignore_index is not None and absolute_c == ignore_index: + if ignore_index is not None and absolute_c == ignore_index and n_class > 1: # ponytail: mirror compute_hausdorff_distance's early-exit for the ignored # channel instead of letting get_edge_surface_distance run on a ground truth # that create_ignore_mask has zeroed out (which produced `inf`, not `nan`, - # and was silently included by do_metric_reduction). + # and was silently included by do_metric_reduction). Only for multi-channel + # (one-hot) input does `ignore_index` name a specific channel to drop entirely; + # for single-channel (label-encoded) input it is a value-based spatial mask + # already applied via create_ignore_mask, and the lone channel is still the + # score being requested, so it must not be forced to nan. asd[b, c] = torch.tensor(float("nan"), device=asd.device) continue yp = y_pred[b, c] diff --git a/tests/metrics/test_surface_distance.py b/tests/metrics/test_surface_distance.py index 9d26fde849..19db9fe9c6 100644 --- a/tests/metrics/test_surface_distance.py +++ b/tests/metrics/test_surface_distance.py @@ -257,6 +257,26 @@ def test_ignore_index_excludes_channel_like_hausdorff(self): sd_result = sd_metric.aggregate() self.assertTrue(torch.isfinite(sd_result).all(), f"aggregate should not be inf/nan, got {sd_result}") + def test_ignore_index_single_channel_does_not_blank_the_only_score(self): + # Regression test: for single-channel (label-encoded) binary input, `ignore_index` + # is a value-based spatial mask (handled by create_ignore_mask before this loop + # runs), not a request to drop an entire one-hot channel. With include_background= + # True and ignore_index=0, `absolute_c == ignore_index` was true for the only + # channel (c=0), so the unconditional early-exit forced that channel to nan for + # every image, which do_metric_reduction's all-nan convention then silently + # reduces to 0 -- reporting a perfect score regardless of the actual prediction. + y = torch.zeros((2, 1, 16, 16)) + y_pred = torch.zeros((2, 1, 16, 16)) + y[:, 0, 4:8, 4:8] = 1 + y_pred[:, 0, 5:9, 5:9] = 1 # off by one voxel, partially overlapping the ground truth + + sd_metric = SurfaceDistanceMetric(include_background=True, ignore_index=0, reduction="mean") + sd_metric(y_pred, y) + result = sd_metric.aggregate() + self.assertFalse(torch.isnan(result).any(), f"expected a real distance, got {result}") + self.assertFalse(torch.isinf(result).any(), f"expected a finite distance, got {result}") + self.assertGreater(result.item(), 0.0, "an imperfect prediction must not score as perfect (0)") + def test_channel_dimensions(self): with self.assertRaises(ValueError): SurfaceDistanceMetric(per_component=True)(torch.ones([3, 3, 144, 144]), torch.ones([3, 3, 144, 144])) From 76f2071ac3509332fd541e8facd0c7748fd41118 Mon Sep 17 00:00:00 2001 From: Siddhardha Nanda <99672439+SID-6921@users.noreply.github.com> Date: Tue, 10 Nov 2026 19:54:57 -0500 Subject: [PATCH 3/3] fix(metrics): base the single-vs-multichannel ignore_index guard on the pre-strip channel count CodeRabbit (round 3) caught that the n_class > 1 guard added in the previous commit is itself computed AFTER ignore_background has already stripped the background channel when include_background=False. A genuinely two-channel one-hot input (background + foreground) therefore collapses to n_class == 1 post-strip, so the guard wrongly treated it as the single-channel/label-encoded case and skipped the early-exit -- reproducing the exact inf-leak this PR exists to fix, just via a different channel-count path. Confirmed with a repro: include_background=False, ignore_index=1 on a 2-channel one-hot input, with a false-positive foreground prediction outside the true foreground region, produced aggregate() == inf. Fix: capture orig_n_channels = y_pred.shape[1] before the ignore_background strip, and use that (not the post-strip n_class) to decide whether ignore_index names a real one-hot channel. Added test_ignore_index_two_channel_exclude_background_still_excludes_channel, which fails (inf leaks into both the buffer and the aggregate) against the previous n_class-based guard and passes with the orig_n_channels fix. Signed-off-by: Siddhardha Nanda <99672439+SID-6921@users.noreply.github.com> --- monai/metrics/surface_distance.py | 11 ++++++++++- tests/metrics/test_surface_distance.py | 23 +++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/monai/metrics/surface_distance.py b/monai/metrics/surface_distance.py index 05af5c7949..7899d1e702 100644 --- a/monai/metrics/surface_distance.py +++ b/monai/metrics/surface_distance.py @@ -209,6 +209,15 @@ def compute_average_surface_distance( per_component: whether to compute the Surface Distance on a per-connected component basis. Defaults to ``False``. """ + # ponytail: capture the channel count before any background-stripping. n_class below + # is recomputed post-strip and, for include_background=False, undercounts how many + # channels the caller actually passed in (e.g. a true 2-channel one-hot input becomes + # n_class==1 after dropping the background channel) -- using the post-strip count to + # decide whether ignore_index names a real one-hot channel wrongly treated a genuinely + # multi-channel ignore_index as the single-channel/label-encoded case, letting the + # ignored channel's (masked-to-empty) ground truth leak `inf` into the aggregate again. + orig_n_channels = y_pred.shape[1] + if not include_background: y_pred, y = ignore_background(y_pred=y_pred, y=y) @@ -228,7 +237,7 @@ def compute_average_surface_distance( for b, c in np.ndindex(batch_size, n_class): absolute_c = c + class_offset - if ignore_index is not None and absolute_c == ignore_index and n_class > 1: + if ignore_index is not None and absolute_c == ignore_index and orig_n_channels > 1: # ponytail: mirror compute_hausdorff_distance's early-exit for the ignored # channel instead of letting get_edge_surface_distance run on a ground truth # that create_ignore_mask has zeroed out (which produced `inf`, not `nan`, diff --git a/tests/metrics/test_surface_distance.py b/tests/metrics/test_surface_distance.py index 19db9fe9c6..b5a7e3ace7 100644 --- a/tests/metrics/test_surface_distance.py +++ b/tests/metrics/test_surface_distance.py @@ -277,6 +277,29 @@ def test_ignore_index_single_channel_does_not_blank_the_only_score(self): self.assertFalse(torch.isinf(result).any(), f"expected a finite distance, got {result}") self.assertGreater(result.item(), 0.0, "an imperfect prediction must not score as perfect (0)") + def test_ignore_index_two_channel_exclude_background_still_excludes_channel(self): + # Regression test: n_class is computed AFTER ignore_background strips the + # background channel (for include_background=False), so a genuinely two-channel + # one-hot input (background + foreground) collapses to n_class == 1 post-strip. + # Guarding the early-exit on that post-strip count wrongly treated this as the + # single-channel/label-encoded case and skipped it, letting the ignored + # (masked-to-empty) foreground channel's ground truth leak `inf` into the + # aggregate again whenever the prediction had a false positive outside the true + # foreground region. The guard must use the channel count from BEFORE stripping. + y = torch.zeros((2, 2, 16, 16)) + y_pred = torch.zeros((2, 2, 16, 16)) + y[:, 1, 4:8, 4:8] = 1 + y[:, 0] = 1 - y[:, 1] + y_pred[:, 1, 10:14, 10:14] = 1 # false positive entirely outside the true foreground + y_pred[:, 0] = 1 - y_pred[:, 1] + + sd_metric = SurfaceDistanceMetric(include_background=False, ignore_index=1, reduction="mean") + sd_metric(y_pred, y) + sd_buffer = sd_metric.get_buffer() + self.assertTrue(torch.isnan(sd_buffer).all(), f"ignored channel should be nan, got {sd_buffer}") + result = sd_metric.aggregate() + self.assertTrue(torch.isfinite(result).all(), f"aggregate should not be inf, got {result}") + def test_channel_dimensions(self): with self.assertRaises(ValueError): SurfaceDistanceMetric(per_component=True)(torch.ones([3, 3, 144, 144]), torch.ones([3, 3, 144, 144]))