feat(losses/metrics): implement ignore_index support across dice and …#8757
Open
Rusheel86 wants to merge 9 commits intoProject-MONAI:devfrom
Open
feat(losses/metrics): implement ignore_index support across dice and …#8757Rusheel86 wants to merge 9 commits intoProject-MONAI:devfrom
Rusheel86 wants to merge 9 commits intoProject-MONAI:devfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request adds native Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Rusheel86
force-pushed
the
feat-ignore-index-support
branch
6 times, most recently
from
7177b8d to
b74ef93
Compare
…ated unit tests Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Rusheel86
force-pushed
the
feat-ignore-index-support
branch
from
b74ef93 to
f2caaf8
Compare
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
I, Rusheel Sharma <rusheelhere@gmail.com>, hereby add my Signed-off-by to this commit: d075009 Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
monai/losses/focal_loss.py (1)
150-154:⚠️ Potential issue | 🔴 Critical
ignore_indexis derived from the wrong target representation.The comparison happens after any
one_hotconversion, so it only works for sentinel-filled tensors. For documented one-hot targets, class IDs are gone andignore_indexnever identifies the ignored voxels. Keep the pre-conversion labels for masking, and fall back to the ignored class channel when the target is already one-hot.Also applies to: 167-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@monai/losses/focal_loss.py` around lines 150 - 154, The ignore_index mask is being computed from the post-conversion target (after one_hot), so when users pass one-hot targets the class IDs are lost and ignored voxels aren’t detected; fix by saving the original target (e.g., orig_target = target) before calling one_hot in the method where self.to_onehot_y is handled, compute the ignore mask from orig_target when it contains integer class labels, and if the supplied target is already one-hot derive the mask from the ignore_index channel (e.g., orig_target[..., ignore_index] or equivalent); apply the same change to the second occurrence of the logic (lines matching the 167-170 block) so both paths correctly detect ignored voxels regardless of whether target was converted or already one-hot.monai/losses/dice.py (1)
169-177:⚠️ Potential issue | 🔴 CriticalBuild the ignore mask from a sanitized label view.
At Line 171 the mask is based on tensor values, so it only works for single-channel sentinel maps. For documented one-hot targets,
ignore_indexnever matches class membership, and withinclude_background=FalsetheC-channel mask no longer broadcasts after Lines 183-185 sliceinputandtarget. Also sanitize ignored labels before Line 177’sone_hot.Also applies to: 179-192
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@monai/losses/dice.py` around lines 169 - 177, Build the ignore mask from a sanitized label view before converting to one-hot: compute a boolean label_mask = (target != self.ignore_index) (or all-True if ignore_index is None), then if self.to_onehot_y is True sanitize target for one_hot by copying target and setting ignored positions to a valid class (e.g., 0) before calling one_hot; after one_hot, expand/reshape label_mask into the channel dimension to match target/prediction (e.g., mask = label_mask.unsqueeze(1).expand_as(target).float()) so the mask works for both single-channel sentinel maps and multi-channel one-hot targets and still broadcasts correctly when include_background slicing happens (apply same mask generation in the later block covering lines ~179-192).monai/metrics/utils.py (3)
316-326:⚠️ Potential issue | 🟡 Minor
ignore_indexparameter unused inget_edge_surface_distance.Parameter at line 325 is accepted but not forwarded to
get_mask_edgesat line 355 or used elsewhere in the function.🛠️ Forward ignore_index to get_mask_edges
- edge_results = get_mask_edges(y_pred, y, crop=True, spacing=edges_spacing, always_return_as_numpy=False) + edge_results = get_mask_edges(y_pred, y, crop=True, spacing=edges_spacing, always_return_as_numpy=False, ignore_index=ignore_index)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@monai/metrics/utils.py` around lines 316 - 326, The get_edge_surface_distance function accepts ignore_index but never forwards it to get_mask_edges or uses it; update get_edge_surface_distance to pass the ignore_index argument through when calling get_mask_edges (the call site around get_mask_edges in get_edge_surface_distance) so masked edge computation respects ignore_index, and ensure any signatures (get_mask_edges invocation) and downstream uses handle a None value appropriately.
265-271:⚠️ Potential issue | 🟡 Minor
maskparameter added but unused.The
maskparameter at line 270 is not used inget_surface_distance. The function computes distances based on edges but doesn't apply the mask. Remove or implement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@monai/metrics/utils.py` around lines 265 - 271, get_surface_distance currently accepts a mask parameter but never uses it; either remove the parameter or apply it to limit where surface distances are computed. To fix, update get_surface_distance so that before computing edges (from seg_pred and seg_gt) you validate and cast mask to a boolean array matching seg_pred/seg_gt shape and then apply it to zero-out/ignore voxels outside the mask (e.g., mask the inputs seg_pred and seg_gt or filter edge point sets) so that subsequent edge extraction and distance calculations only consider masked regions; keep the existing logic for distance_metric and spacing unchanged and ensure mask handling is documented in the function signature and tests.
161-169:⚠️ Potential issue | 🟡 Minor
ignore_indexparameter is unused and undocumented.The parameter at line 168 is never referenced in the function body and missing from the docstring's Args section. Either implement the functionality and document it, or remove the parameter to keep the API honest.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@monai/metrics/utils.py` around lines 161 - 169, The ignore_index parameter in get_mask_edges is never used and not documented; remove it from the function signature and any related type hints, delete its mention in the docstring Args section, and update any call sites/tests that pass ignore_index to stop supplying it; alternatively if you prefer keeping behavior, implement logic inside get_mask_edges to mask out label==ignore_index from seg_gt/seg_pred before processing and document the parameter in the docstring—choose one approach and make matching changes to callers and tests for consistency.
🧹 Nitpick comments (2)
monai/metrics/hausdorff_distance.py (1)
199-207: Redundant masking when bothignore_indexandmaskare used.At line 105-107,
y_predandyare already masked. Then at lines 200-202,ypandytare masked again withvalid_mask. Themask[b, 0]is also passed toget_edge_surface_distanceat line 215. This triple application may be intentional for robustness but adds overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@monai/metrics/hausdorff_distance.py` around lines 199 - 207, The code redundantly reapplies masking: y_pred/y are already masked earlier (mask[b, 0]) yet yp/yt are masked again with valid_mask before computing distances and the same mask is later passed into get_edge_surface_distance; remove the duplicated masking to avoid overhead by eliminating the yp = yp * valid_mask and yt = yt * valid_mask lines (or skip the earlier mask and only apply a single, consistent mask) and ensure the mask passed to get_edge_surface_distance is the same single mask used to prepare yp/yt; update references in the loop that set hd[b, c] and the early all-ignored check to use that single mask (valid_mask or mask[b, 0]) so masking occurs exactly once.monai/metrics/generalized_dice.py (1)
198-204: Per-batch loop for inf replacement is functional but could be vectorized.Works correctly but iterating per batch may be slower for large batches.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@monai/metrics/generalized_dice.py` around lines 198 - 204, Loop over batches replacing infs in w_full is correct but can be vectorized: compute inf_mask = torch.isinf(w_full), create temp = w_full.masked_fill(inf_mask, 0), compute per-batch max_w = temp.view(w_full.shape[0], -1).max(dim=1).values, build per-batch replacement values = torch.where(max_w > 0, max_w, torch.tensor(1.0, device=w_full.device)), expand/broadcast those replacements to the shape of w_full and assign them into w_full at inf_mask; update the code replacing the per-batch for-loop (variables: w_full, batch_w, infs) with this vectorized sequence so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@monai/losses/focal_loss.py`:
- Around line 167-171: The current implementation applies the ignore mask to
logits/targets (input/target) which still yields a nonzero BCE/focal term for
masked entries; instead, compute the elementwise focal/BCE loss normally (in the
sigmoid and other paths) and then multiply that per-element loss by mask to zero
out ignored voxels, and when reducing use sum(loss * mask) / mask.sum() (or
clamp minimal denom) so ignored elements don't contribute to numerator or
denominator; update the forward method in FocalLoss (and the other masked blocks
referenced around the sigmoid branch and the regions you noted at 183-185 and
207-218) to apply mask to the computed loss tensor and perform masked reduction
rather than masking input/target or using .mean() over masked values.
In `@monai/losses/tversky.py`:
- Around line 135-136: original_target is preserved but one_hot is called on the
raw target which can contain out-of-range sentinel values (e.g., 255) causing
one_hot to fail; before calling one_hot(target, num_classes=n_pred_ch) replace
ignored/sentinel labels in target with a valid class id (e.g., 0 or another safe
class index) then call one_hot using that sanitized target, and continue to
derive the ignore mask from original_target (not the sanitized target) when
constructing the mask and applying it in the subsequent loss logic (refer to
original_target, target, one_hot, n_pred_ch, and the mask construction used in
the block around lines 138-148).
In `@monai/losses/unified_focal_loss.py`:
- Around line 167-172: The mean-reduction branch incorrectly divides loss.sum()
by spatial_mask.sum(), undercounting channels (loss has shape [B,2,H,W]); update
the normalization to account for both channel elements by either applying the
spatial mask to loss (e.g., expand spatial_mask to match loss with
spatial_mask.unsqueeze(1).expand_as(loss)), summing the masked loss and dividing
by the masked-element count, or divide loss.sum() by (spatial_mask.sum() *
loss.size(1)). Make the change in the block handling self.reduction ==
LossReduction.MEAN.value and self.ignore_index is not None (affecting variables
loss, spatial_mask, back_ce, fore_ce).
- Around line 71-77: The current mask uses (torch.sum(y_true, dim=1) > 0) which
does not respect ignore_index; update the ignore handling to build the mask from
the original label indices or from the explicit ignored-class channel instead of
summing one-hot channels. Concretely: when ignore_index is set, if you have
access to the original label tensor (e.g., labels / y_true_indices) create
valid_mask = (labels != self.ignore_index).unsqueeze(1) and expand it to y_true
shape; otherwise (when y_true is one-hot) build the mask from the ignored class
channel as valid_mask = 1 - y_true[:, self.ignore_index:self.ignore_index+1,
...] and use that to mask y_pred and other computations (replace spatial_mask
usage in this file and in AsymmetricFocalTverskyLoss code paths).
- Around line 157-160: The ignore_index handling currently builds spatial_mask
from torch.sum(y_true, dim=1) which fails when self.to_onehot_y is False; update
the block in AsymmetricFocalLoss (unified_focal_loss.py) to branch on
self.to_onehot_y: when to_onehot_y is False compute spatial_mask = (y_true !=
self.ignore_index).unsqueeze(1).float() (so label tensors drop ignored voxels),
otherwise keep the existing one-hot style mask (e.g., spatial_mask =
(torch.sum(y_true, dim=1, keepdim=True) > 0).float()); then apply that
spatial_mask to cross_entropy as before.
In `@monai/metrics/generalized_dice.py`:
- Around line 157-161: The current ignore_index mask in
monai/metrics/generalized_dice.py compares a one-hot y to an integer and always
yields True; change the masking to detect one-hot class vectors using n_class:
if ignore_index >= n_class set mask = (y.sum(dim=1, keepdim=True) > 0).float(),
else set mask = (1.0 - y[:, ignore_index:ignore_index+1]).float(); then apply
that mask to y_pred and y as before (y_pred = y_pred * mask; y = y * mask). Use
the existing variables y, y_pred, ignore_index and n_class to implement this
branching so one-hot inputs are masked correctly.
In `@monai/metrics/hausdorff_distance.py`:
- Around line 151-152: The docstring for hausdorff_distance is missing
documentation for the mask parameter; update the function's docstring to
describe mask: its type (torch.Tensor | None), expected shape (same spatial
shape as y_pred/y), semantics (binary or boolean tensor where True/non-zero
marks voxels to include in the metric; if None, no spatial masking is applied),
how it interacts with ignore_index (e.g., masked-out voxels are excluded before
applying ignore_index), and any dtype/format expectations (boolean or 0/1
integer). Mention that mask applies to both prediction and ground-truth when
computing distances and that mask and ignore_index can be used together (specify
precedence if relevant).
In `@monai/metrics/meaniou.py`:
- Around line 146-151: compute_iou's ignore_index handling assumes label-encoded
y but must support one-hot y; change the mask logic in compute_iou to detect
one-hot (y.dim()==y_pred.dim() and y.shape[1]==y_pred.shape[1] or otherwise by
checking channel count) and, when one-hot, build a [B,1,...] mask from y[:,
ignore_index:ignore_index+1] (e.g., mask = 1 - y[:,
ignore_index:ignore_index+1]) then expand_as(y_pred) and apply it to y_pred; for
label-encoded y keep the existing mask = (y != ignore_index).float() and replace
y values with 0 via torch.where(y == ignore_index, torch.tensor(0,
device=y.device), y) only in that branch so one-hot y is not altered
incorrectly.
In `@monai/metrics/surface_dice.py`:
- Around line 223-227: The current one-hot mask logic using (y !=
ignore_index).all(dim=1, keepdim=True) is incorrect for one-hot encoded labels;
replace it by constructing a class-based mask from the one-hot channel: build
mask = (1 - y[:, ignore_index:ignore_index+1]).float() (preserving a [B,1,...]
shape) and then multiply y_pred and y by that mask; update the block that checks
ignore_index and applies masking to y_pred and y (referencing y, y_pred, and
ignore_index) so ignored-class voxels are zeroed correctly.
In `@monai/metrics/surface_distance.py`:
- Around line 174-176: The current conditional skips calling ignore_background
when include_background is False and ignore_index == 0, causing background
removal to be bypassed unintentionally; update the logic in surface_distance.py
so that when include_background is False you always call
ignore_background(y_pred=y_pred, y=y) (remove or change the ignore_index check),
or alternatively make the behavior explicit by documenting that ignore_index==0
preserves background; reference the include_background flag, ignore_index
variable, and the ignore_background(y_pred=y_pred, y=y) call when making the
change.
In `@monai/metrics/utils.py`:
- Around line 358-364: The mask-slicing branch currently only runs when
edge_results[2] is a tuple, so in subvoxel mode (when spacing is provided and
edge_results[2] is an area tensor) the mask is never applied; update the code
around edge_results/ mask handling so it handles both cases: if edge_results[2]
is a tuple extract slices and do mask = mask[slices], otherwise skip slicing and
just move/cast the mask to the edges_pred device (mask =
mask.to(edges_pred.device).bool()), then apply edges_pred = edges_pred & mask
and edges_gt = edges_gt & mask. Ensure you reference edge_results, mask,
edges_pred and edges_gt when making the change.
In `@tests/losses/test_ignore_index_losses.py`:
- Around line 22-27: The current TEST_CASES only checks invariance with ignored
indices but not that ignored voxels are truly excluded; update the tests in
tests/losses/test_ignore_index_losses.py to compute each loss twice—(1) the
existing full-volume loss with ignore_index set and (2) a reference loss
computed only on the non-ignored region (mask out ignored voxels from y_true and
y_pred and/or slice to the non-ignored voxels) and assert those values match—so
constant penalties from ignored voxels would fail; extend TEST_CASES to include
both to_onehot_y=True and an already-one-hot label case for each Loss class
(DiceLoss, FocalLoss, TverskyLoss, AsymmetricUnifiedFocalLoss) and ensure the
test covers both softmax/sigmoid variants referenced in the tuple entries.
In `@tests/metrics/test_ignore_index_metrics.py`:
- Around line 48-50: The class-level `@unittest.skipUnless`(has_scipy, ...) is
incorrectly skipping metrics that don't require SciPy; update the test split so
non-SciPy metrics run regardless. Remove or change the decorator on
TestIgnoreIndexMetrics and have it use `@parameterized.expand`(TEST_METRICS) only,
then create a separate test class (e.g., TestIgnoreIndexSurfaceMetrics)
decorated with `@unittest.skipUnless`(has_scipy, ...) that uses
`@parameterized.expand`(SCIPY_METRICS) so only the surface-distance tests are
gated on has_scipy.
- Around line 63-66: The test currently fills all channels with 255 which never
triggers the class-based one-hot path; change the bottom-half sentinel
assignment to a proper one-hot vector for the ignored class so the masking logic
is exercised—specifically, update the tensor y (created in the test) so that for
the ignored class channel (referenced as the channel index used by the
metric/ignored_class constant) you set y[:, ignored_channel, 2:4, :] = 1 and
ensure the other channels in that region remain 0; keep the top-half valid
labels (y[:, 1, 0:2, 0:2] = 1.0) intact so both valid and ignored-class regions
are present and the class-based one-hot branch in the metric (the one handling
one-hot label tensors) is covered.
---
Outside diff comments:
In `@monai/losses/dice.py`:
- Around line 169-177: Build the ignore mask from a sanitized label view before
converting to one-hot: compute a boolean label_mask = (target !=
self.ignore_index) (or all-True if ignore_index is None), then if
self.to_onehot_y is True sanitize target for one_hot by copying target and
setting ignored positions to a valid class (e.g., 0) before calling one_hot;
after one_hot, expand/reshape label_mask into the channel dimension to match
target/prediction (e.g., mask =
label_mask.unsqueeze(1).expand_as(target).float()) so the mask works for both
single-channel sentinel maps and multi-channel one-hot targets and still
broadcasts correctly when include_background slicing happens (apply same mask
generation in the later block covering lines ~179-192).
In `@monai/losses/focal_loss.py`:
- Around line 150-154: The ignore_index mask is being computed from the
post-conversion target (after one_hot), so when users pass one-hot targets the
class IDs are lost and ignored voxels aren’t detected; fix by saving the
original target (e.g., orig_target = target) before calling one_hot in the
method where self.to_onehot_y is handled, compute the ignore mask from
orig_target when it contains integer class labels, and if the supplied target is
already one-hot derive the mask from the ignore_index channel (e.g.,
orig_target[..., ignore_index] or equivalent); apply the same change to the
second occurrence of the logic (lines matching the 167-170 block) so both paths
correctly detect ignored voxels regardless of whether target was converted or
already one-hot.
In `@monai/metrics/utils.py`:
- Around line 316-326: The get_edge_surface_distance function accepts
ignore_index but never forwards it to get_mask_edges or uses it; update
get_edge_surface_distance to pass the ignore_index argument through when calling
get_mask_edges (the call site around get_mask_edges in
get_edge_surface_distance) so masked edge computation respects ignore_index, and
ensure any signatures (get_mask_edges invocation) and downstream uses handle a
None value appropriately.
- Around line 265-271: get_surface_distance currently accepts a mask parameter
but never uses it; either remove the parameter or apply it to limit where
surface distances are computed. To fix, update get_surface_distance so that
before computing edges (from seg_pred and seg_gt) you validate and cast mask to
a boolean array matching seg_pred/seg_gt shape and then apply it to
zero-out/ignore voxels outside the mask (e.g., mask the inputs seg_pred and
seg_gt or filter edge point sets) so that subsequent edge extraction and
distance calculations only consider masked regions; keep the existing logic for
distance_metric and spacing unchanged and ensure mask handling is documented in
the function signature and tests.
- Around line 161-169: The ignore_index parameter in get_mask_edges is never
used and not documented; remove it from the function signature and any related
type hints, delete its mention in the docstring Args section, and update any
call sites/tests that pass ignore_index to stop supplying it; alternatively if
you prefer keeping behavior, implement logic inside get_mask_edges to mask out
label==ignore_index from seg_gt/seg_pred before processing and document the
parameter in the docstring—choose one approach and make matching changes to
callers and tests for consistency.
---
Nitpick comments:
In `@monai/metrics/generalized_dice.py`:
- Around line 198-204: Loop over batches replacing infs in w_full is correct but
can be vectorized: compute inf_mask = torch.isinf(w_full), create temp =
w_full.masked_fill(inf_mask, 0), compute per-batch max_w =
temp.view(w_full.shape[0], -1).max(dim=1).values, build per-batch replacement
values = torch.where(max_w > 0, max_w, torch.tensor(1.0, device=w_full.device)),
expand/broadcast those replacements to the shape of w_full and assign them into
w_full at inf_mask; update the code replacing the per-batch for-loop (variables:
w_full, batch_w, infs) with this vectorized sequence so behavior remains
identical.
In `@monai/metrics/hausdorff_distance.py`:
- Around line 199-207: The code redundantly reapplies masking: y_pred/y are
already masked earlier (mask[b, 0]) yet yp/yt are masked again with valid_mask
before computing distances and the same mask is later passed into
get_edge_surface_distance; remove the duplicated masking to avoid overhead by
eliminating the yp = yp * valid_mask and yt = yt * valid_mask lines (or skip the
earlier mask and only apply a single, consistent mask) and ensure the mask
passed to get_edge_surface_distance is the same single mask used to prepare
yp/yt; update references in the loop that set hd[b, c] and the early all-ignored
check to use that single mask (valid_mask or mask[b, 0]) so masking occurs
exactly once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de4d36bb-f303-47c9-bc69-6339e0ccdc88
📒 Files selected for processing (14)
monai/losses/dice.pymonai/losses/focal_loss.pymonai/losses/tversky.pymonai/losses/unified_focal_loss.pymonai/metrics/confusion_matrix.pymonai/metrics/generalized_dice.pymonai/metrics/hausdorff_distance.pymonai/metrics/meandice.pymonai/metrics/meaniou.pymonai/metrics/surface_dice.pymonai/metrics/surface_distance.pymonai/metrics/utils.pytests/losses/test_ignore_index_losses.pytests/metrics/test_ignore_index_metrics.py
Comment on lines
+167
to
+171
| if self.ignore_index is not None: | ||
| mask = (target != self.ignore_index).float() | ||
| input = input * mask | ||
| target = target * mask | ||
|
|
Contributor
There was a problem hiding this comment.
Mask the focal loss, not the logits.
In the sigmoid path, input = 0 and target = 0 still produce a positive BCE/focal term, so ignored voxels are turned into background loss instead of being excluded. loss.mean() also continues to divide by ignored elements. Apply the mask to the elementwise loss, then reduce over valid elements only.
Also applies to: 183-185, 207-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@monai/losses/focal_loss.py` around lines 167 - 171, The current
implementation applies the ignore mask to logits/targets (input/target) which
still yields a nonzero BCE/focal term for masked entries; instead, compute the
elementwise focal/BCE loss normally (in the sigmoid and other paths) and then
multiply that per-element loss by mask to zero out ignored voxels, and when
reducing use sum(loss * mask) / mask.sum() (or clamp minimal denom) so ignored
elements don't contribute to numerator or denominator; update the forward method
in FocalLoss (and the other masked blocks referenced around the sigmoid branch
and the regions you noted at 183-185 and 207-218) to apply mask to the computed
loss tensor and perform masked reduction rather than masking input/target or
using .mean() over masked values.
Comment on lines
+135
to
136
| original_target = target | ||
| target = one_hot(target, num_classes=n_pred_ch) |
Contributor
There was a problem hiding this comment.
Sanitize ignored labels before one_hot.
original_target is saved at Line 135, but Line 136 still one-hot encodes the raw labels. If callers use an out-of-range sentinel such as 255, conversion fails before the mask at Lines 138-148 runs. Replace ignored voxels with a valid class ID before one_hot, then derive the mask from original_target.
Also applies to: 138-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@monai/losses/tversky.py` around lines 135 - 136, original_target is preserved
but one_hot is called on the raw target which can contain out-of-range sentinel
values (e.g., 255) causing one_hot to fail; before calling one_hot(target,
num_classes=n_pred_ch) replace ignored/sentinel labels in target with a valid
class id (e.g., 0 or another safe class index) then call one_hot using that
sanitized target, and continue to derive the ignore mask from original_target
(not the sanitized target) when constructing the mask and applying it in the
subsequent loss logic (refer to original_target, target, one_hot, n_pred_ch, and
the mask construction used in the block around lines 138-148).
Comment on lines
+71
to
+77
| # Handle ignore_index: | ||
| mask = torch.ones_like(y_true) | ||
| if self.ignore_index is not None: | ||
| # Identify valid pixels: where at least one channel is 1 | ||
| spatial_mask = (torch.sum(y_true, dim=1, keepdim=True) > 0).float() | ||
| mask = spatial_mask.expand_as(y_true) | ||
| y_pred = y_pred * mask |
Contributor
There was a problem hiding this comment.
ignore_index is not used to build this mask.
sum(y_true, dim=1) > 0 marks every normal one-hot voxel as valid, regardless of which class should be ignored, and it also keeps sentinel-filled ignored voxels (255 + 255 > 0). The standalone AsymmetricFocalTverskyLoss path therefore still includes ignored regions. Build the mask from the original labels, or from the ignored class channel.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@monai/losses/unified_focal_loss.py` around lines 71 - 77, The current mask
uses (torch.sum(y_true, dim=1) > 0) which does not respect ignore_index; update
the ignore handling to build the mask from the original label indices or from
the explicit ignored-class channel instead of summing one-hot channels.
Concretely: when ignore_index is set, if you have access to the original label
tensor (e.g., labels / y_true_indices) create valid_mask = (labels !=
self.ignore_index).unsqueeze(1) and expand it to y_true shape; otherwise (when
y_true is one-hot) build the mask from the ignored class channel as valid_mask =
1 - y_true[:, self.ignore_index:self.ignore_index+1, ...] and use that to mask
y_pred and other computations (replace spatial_mask usage in this file and in
AsymmetricFocalTverskyLoss code paths).
Comment on lines
+157
to
+160
| if self.ignore_index is not None: | ||
| spatial_mask = (torch.sum(y_true, dim=1, keepdim=True) > 0).float() | ||
| cross_entropy = cross_entropy * spatial_mask | ||
|
|
Contributor
There was a problem hiding this comment.
The default AsymmetricFocalLoss path still includes ignored voxels.
With to_onehot_y=False, this again uses sum(y_true) > 0, so one-hot targets never drop a specific class and sentinel-filled ignored regions stay valid. Since that is the default mode, ignore_index is ineffective for the standalone loss.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@monai/losses/unified_focal_loss.py` around lines 157 - 160, The ignore_index
handling currently builds spatial_mask from torch.sum(y_true, dim=1) which fails
when self.to_onehot_y is False; update the block in AsymmetricFocalLoss
(unified_focal_loss.py) to branch on self.to_onehot_y: when to_onehot_y is False
compute spatial_mask = (y_true != self.ignore_index).unsqueeze(1).float() (so
label tensors drop ignored voxels), otherwise keep the existing one-hot style
mask (e.g., spatial_mask = (torch.sum(y_true, dim=1, keepdim=True) >
0).float()); then apply that spatial_mask to cross_entropy as before.
Comment on lines
+167
to
+172
| loss = torch.stack([back_ce, fore_ce], dim=1) # [B, 2, H, W] | ||
| if self.reduction == LossReduction.MEAN.value: | ||
| if self.ignore_index is not None: | ||
| # Normalize by the number of non-ignored pixels | ||
| return loss.sum() / spatial_mask.sum().clamp(min=1e-5) | ||
| return loss.mean() |
Contributor
There was a problem hiding this comment.
mean reduction is scaled by pixels, not valid elements.
loss.sum() / spatial_mask.sum() is 2x larger than loss.mean() when nothing is ignored because loss has two channels but the denominator counts only pixels. Divide by the expanded mask sum, or average loss after masking it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@monai/losses/unified_focal_loss.py` around lines 167 - 172, The
mean-reduction branch incorrectly divides loss.sum() by spatial_mask.sum(),
undercounting channels (loss has shape [B,2,H,W]); update the normalization to
account for both channel elements by either applying the spatial mask to loss
(e.g., expand spatial_mask to match loss with
spatial_mask.unsqueeze(1).expand_as(loss)), summing the masked loss and dividing
by the masked-element count, or divide loss.sum() by (spatial_mask.sum() *
loss.size(1)). Make the change in the block handling self.reduction ==
LossReduction.MEAN.value and self.ignore_index is not None (affecting variables
loss, spatial_mask, back_ce, fore_ce).
Comment on lines
174
to
+176
| if not include_background: | ||
| y_pred, y = ignore_background(y_pred=y_pred, y=y) | ||
| if ignore_index != 0: | ||
| y_pred, y = ignore_background(y_pred=y_pred, y=y) |
Contributor
There was a problem hiding this comment.
Edge case: ignore_index=0 skips background removal entirely.
When ignore_index=0, line 175 evaluates to False, so ignore_background is never called even if include_background=False. This means setting ignore_index=0 overrides include_background=False, which may be intentional but should be documented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@monai/metrics/surface_distance.py` around lines 174 - 176, The current
conditional skips calling ignore_background when include_background is False and
ignore_index == 0, causing background removal to be bypassed unintentionally;
update the logic in surface_distance.py so that when include_background is False
you always call ignore_background(y_pred=y_pred, y=y) (remove or change the
ignore_index check), or alternatively make the behavior explicit by documenting
that ignore_index==0 preserves background; reference the include_background
flag, ignore_index variable, and the ignore_background(y_pred=y_pred, y=y) call
when making the change.
Comment on lines
+358
to
+364
| if mask is not None: | ||
| if len(edge_results) > 2 and isinstance(edge_results[2], tuple): | ||
| slices = edge_results[2] | ||
| mask = mask[slices] | ||
| mask = mask.to(edges_pred.device).bool() | ||
| edges_pred = edges_pred & mask | ||
| edges_gt = edges_gt & mask |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check get_mask_edges return structure
rg -n "return.*edges" monai/metrics/utils.py -B 2 -A 2Repository: Project-MONAI/MONAI
Length of output: 859
🏁 Script executed:
rg -n "def get_mask_edges" monai/metrics/utils.py -A 50Repository: Project-MONAI/MONAI
Length of output: 2700
🏁 Script executed:
sed -n '340,365p' monai/metrics/utils.pyRepository: Project-MONAI/MONAI
Length of output: 1280
🏁 Script executed:
rg -n "return" monai/metrics/utils.py | grep -A 2 -B 2 "161\|162\|163\|164\|165"Repository: Project-MONAI/MONAI
Length of output: 45
🏁 Script executed:
sed -n '161,330p' monai/metrics/utils.pyRepository: Project-MONAI/MONAI
Length of output: 8350
🏁 Script executed:
# Check if CropForegroundD or any other part returns slices
rg -n "slices" monai/metrics/utils.pyRepository: Project-MONAI/MONAI
Length of output: 139
🏁 Script executed:
rg -n "class CropForegroundD" monai/Repository: Project-MONAI/MONAI
Length of output: 45
Mask slicing condition will never execute in subvoxel mode.
Line 359 checks isinstance(edge_results[2], tuple), but when spacing is provided, edge_results[2] contains an area tensor, not slices. The mask application at lines 360-361 will be skipped, leaving masks unapplied in subvoxel mode.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@monai/metrics/utils.py` around lines 358 - 364, The mask-slicing branch
currently only runs when edge_results[2] is a tuple, so in subvoxel mode (when
spacing is provided and edge_results[2] is an area tensor) the mask is never
applied; update the code around edge_results/ mask handling so it handles both
cases: if edge_results[2] is a tuple extract slices and do mask = mask[slices],
otherwise skip slicing and just move/cast the mask to the edges_pred device
(mask = mask.to(edges_pred.device).bool()), then apply edges_pred = edges_pred &
mask and edges_gt = edges_gt & mask. Ensure you reference edge_results, mask,
edges_pred and edges_gt when making the change.
Comment on lines
+22
to
+27
| # Defining test cases: (LossClass, args) | ||
| TEST_CASES = [ | ||
| (DiceLoss, {"sigmoid": True}), | ||
| (FocalLoss, {"use_softmax": False}), | ||
| (TverskyLoss, {"sigmoid": True}), | ||
| (AsymmetricUnifiedFocalLoss, {}), |
Contributor
There was a problem hiding this comment.
This only checks invariance, not actual exclusion.
A loss can still add a constant penalty from ignored voxels and pass this test as long as both inputs collapse to the same value there. Compare against a reference loss computed on just the non-ignored region, and add to_onehot_y=True / already-one-hot cases so the new paths are covered. As per coding guidelines, "Ensure new or modified definitions will be covered by existing or new unit tests."
Also applies to: 33-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/losses/test_ignore_index_losses.py` around lines 22 - 27, The current
TEST_CASES only checks invariance with ignored indices but not that ignored
voxels are truly excluded; update the tests in
tests/losses/test_ignore_index_losses.py to compute each loss twice—(1) the
existing full-volume loss with ignore_index set and (2) a reference loss
computed only on the non-ignored region (mask out ignored voxels from y_true and
y_pred and/or slice to the non-ignored voxels) and assert those values match—so
constant penalties from ignored voxels would fail; extend TEST_CASES to include
both to_onehot_y=True and an already-one-hot label case for each Loss class
(DiceLoss, FocalLoss, TverskyLoss, AsymmetricUnifiedFocalLoss) and ensure the
test covers both softmax/sigmoid variants referenced in the tuple entries.
Comment on lines
+48
to
+50
| @unittest.skipUnless(has_scipy, "Scipy required for surface metrics") | ||
| class TestIgnoreIndexMetrics(unittest.TestCase): | ||
| @parameterized.expand(TEST_METRICS + SCIPY_METRICS) |
Contributor
There was a problem hiding this comment.
Don't gate the non-SciPy metrics behind SciPy.
This decorator skips DiceMetric, MeanIoU, GeneralizedDiceScore, and ConfusionMatrixMetric whenever SciPy is absent, even though they do not depend on it. Split the surface-distance cases into a separate skipped class, or skip only those parameter sets. As per coding guidelines, "Ensure new or modified definitions will be covered by existing or new unit tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/metrics/test_ignore_index_metrics.py` around lines 48 - 50, The
class-level `@unittest.skipUnless`(has_scipy, ...) is incorrectly skipping metrics
that don't require SciPy; update the test split so non-SciPy metrics run
regardless. Remove or change the decorator on TestIgnoreIndexMetrics and have it
use `@parameterized.expand`(TEST_METRICS) only, then create a separate test class
(e.g., TestIgnoreIndexSurfaceMetrics) decorated with
`@unittest.skipUnless`(has_scipy, ...) that uses
`@parameterized.expand`(SCIPY_METRICS) so only the surface-distance tests are
gated on has_scipy.
Comment on lines
+63
to
+66
| # Target: Top half is valid (0/1), Bottom half is 255 | ||
| y = torch.zeros((1, 2, 4, 4)) | ||
| y[:, 1, 0:2, 0:2] = 1.0 | ||
| y[:, :, 2:4, :] = 255 |
Contributor
There was a problem hiding this comment.
These fixtures never hit the class-based one-hot path.
Writing 255 into every channel only tests a sentinel-filled tensor. The metric APIs here expect one-hot labels, so add a case where the ignored class channel is 1 and the others are 0; otherwise the new masking logic is not exercised. As per coding guidelines, "Ensure new or modified definitions will be covered by existing or new unit tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/metrics/test_ignore_index_metrics.py` around lines 63 - 66, The test
currently fills all channels with 255 which never triggers the class-based
one-hot path; change the bottom-half sentinel assignment to a proper one-hot
vector for the ignored class so the masking logic is exercised—specifically,
update the tensor y (created in the test) so that for the ignored class channel
(referenced as the channel index used by the metric/ignored_class constant) you
set y[:, ignored_channel, 2:4, :] = 1 and ensure the other channels in that
region remain 0; keep the top-half valid labels (y[:, 1, 0:2, 0:2] = 1.0) intact
so both valid and ignored-class regions are present and the class-based one-hot
branch in the metric (the one handling one-hot label tensors) is covered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8734
Description
This PR introduces comprehensive native support for
ignore_indexin core MONAI losses and metrics, as requested in issue #8734. Theignore_indexparameter allows specific label values (e.g., padding, unlabeled regions, or boundary artifacts) to be excluded from loss and metric calculations which is a critical feature for medical imaging workflows.Implementation Summary
Losses with
ignore_indexsupport:DiceLoss- masks ignored voxels before dice computationFocalLoss- applies masking after one-hot conversionTverskyLoss- consistent masking approach with DiceLossUnifiedFocalLoss- handles ignore_index in binary target conversionMetrics with
ignore_indexsupport:MeanDice&GeneralizedDiceScore- spatial masking before computationMeanIoU- excludes ignored classes from IoU calculationConfusionMatrixMetric- filters ignored indices from confusion matrixHausdorffDistanceMetric- masks boundary computationSurfaceDiceMetric&SurfaceDistanceMetric- handles ignore_index in surface calculationsKey Fixes:
ignore_indextest_ignore_index_losses.pyandtest_ignore_index_metrics.pyTypes of changes
test_ignore_index_losses.py,test_ignore_index_metrics.py)./runtests.sh -f -u --net --coverage./runtests.sh --quick --unittests --disttestsmake htmlcommand in thedocs/folderRelated Issues
Could merge with or addresses similar requirements as #8667 (Ignore Class)