Precarious label map creation when using custom classes

[siddancha]

When using custom classes, the label map is created in this way (copied below). This code fragment is fragile because missing classes are mapped to "-1", but the "-1" is never handled properly.

# dictionary, its keys are the old label ids and its values
# are the new label ids.
# used for changing pixel labels in load_annotations.
self.label_map = {}
for i, c in enumerate(self.CLASSES):
    if c not in class_names:
        self.label_map[i] = -1
    else:
        self.label_map[i] = class_names.index(c)

I've tried to understand how this "-1" label is handled when training. The expected behavior is that this "-1" label should be ignored. However, this behavior is achieved very indirectly and precariously (prone to bugs):

In loading.py:

gt_semantic_seg = mmcv.imfrombytes(
    img_bytes, flag='unchanged',
    backend=self.imdecode_backend).squeeze().astype(np.uint8)

gt_semantic_seg is created with type uint8. Then,

if results.get('label_map', None) is not None:
    for old_id, new_id in results['label_map'].items():
        gt_semantic_seg[gt_semantic_seg == old_id] = new_id

When new_id is -1, uint8 underflows to 255. Then, since the default ignore_index=255 in custom.py, the "-1" label is successfully ignored.

My issue is that this process is not explicit and can be prone to bugs. For example, the uint8 masks are eventually converted to torch.Tensor of type int64, which can contain negative values. The code only works just because an intermediate step happened to use uint8 and the underflow occurred.

I think the code should be improved and the "-1" label should be handled more explicitly, by, for example, always ignoring negative labels in addition to ignoring the (optional) ignore_index label.

[MeowZheng]

Thanks for your bug report, and for other high-quality PR from you. 👍

We also found it is not robust, and modify it in MMSegmentation 1.x

mmsegmentation/mmseg/datasets/basesegdataset.py

Lines 181 to 185 in a115b10

	if c not in new_classes:
	label_map[i] = 255
	else:
	label_map[i] = new_classes.index(c)
	return label_map

In new mmseg, we give up the unstable underflows, just set the default ignore value 255 in decode head of mmseg model.

Do you have any comments about this modification? Are there any potential bugs we haven't thought of?

[siddancha]

Thanks for your prompt replies!

Do you have any comments about this modification? Are there any potential bugs we haven't thought of?

Thanks for pointing out that this has been fixed in 1.x. I think for now, this is an okay fix, but it can be improved.

Currently, at various places in both the master and 1.x branches, ignore_index defaults sometime to 255 (see here), -100 (see here), None (see here), etc. This could lead to a potential bug. For example, if someone wants to ignore the index 1 for some reason and sets ignore_index=1, then 255 will not be ignored. However, logically, the label 255 should always be ignored regardless of which other labels are being ignored, because reduce_zero_label and label_map assume that 255 will be ignored. I think there can be two solutions to deal with this:

1. Just remove the ignore_index option; an index should only be ignored if (a) reduce_zero_label is set to True or (2) labels are remapped via label_map or (3) a subset of the dataset's classes are specified; all these options use 255 to ignore indices anyway. I don't see any other use cases of explicitly setting the ignore_index so it might be better to just remove it and reduce the chance of bugs.
2. Keep ignore_index, but make it a list (or better yet, a set). Always insert 255 into the set along with potentially any other labels.

I think this is not urgent; the current fix you have works for now.

I had no idea that a 1.x branch existed! Thanks for pointing it out. I have a few questions:

1. Do you recommend switching from master to 1.x at this point?
2. When will 1.x be released?
3. When it does release, will all the bug fixes in the current master (e.g. [Fix] Switch order of reduce_zero_label and applying label_map #2500, [Fix] Fix reduce_zero_label in evaluation #2504) be inherited?

In the meantime, I'll create a PR for master to set label_map[i] = 255.

[siddancha]

I created two PRs:

1. Converting -1 to 255 in master: [Fix] Fix ignore class id from -1 to 255 in master #2515.
2. I just realized that 1.x had an unfinished conversion of -1 to 255 introduced in [Fix] Fix ignore class id from -1 to 255 in BaseSegDataset in 1.x #2332. I created a new PR ([Fix] Unfinished label conversion from -1 to 255 in 1.x #2516) to dev-1.x that completes this conversion.

[MeowZheng]

2. Keep ignore_index, but make it a list (or better yet, a set). Always insert 255 into the set along with potentially any other labels.

I think this is not urgent; the current fix you have works for now.

I prefer this solution, and for avoiding bc-breaking, ignore_index will support int and list(or set).
I think this modification will involve loss function implementation, and we will work on it before mmseg 1.x is officially released.

Do you recommend switching from master to 1.x at this point?

yes

When will 1.x be released?

1.x now is pre-released and we will officially release it in Q1 2023

When it does release, will all the bug fixes in the current master (e.g. #2500, #2504) be inherited?
In the meantime, I'll create a PR for master to set label_map[i] = 255.

no. 1.x branch will be renamed as master and original master will be renamed as 0.x when mmseg 1.x is released.
We will deprecate 0.x in 2024, before 2024, we fix bugs of both two branches but support new features first in 1.x rather than 0.x, so we need all your fixes. :)

Here is migration documentation, if you have any questions or suggestions, just let us know,
and we appreciate any feedback from the community.

[siddancha]

Fixed by #2515 that is now merged.