open-mmlab / mmdetection

OpenMMLab Detection Toolbox and Benchmark
https://mmdetection.readthedocs.io
Apache License 2.0
29.06k stars 9.37k forks source link

CocoDataset uses annotation 'area' property to decide whether bbox should be used #4436

Open waddington-ou-phd-1 opened 3 years ago

waddington-ou-phd-1 commented 3 years ago

Thanks for your error report and we appreciate it a lot.

Describe the bug

Line 146 and 147 of mmdet/datasets/coco.py https://github.com/open-mmlab/mmdetection/blob/master/mmdet/datasets/coco.py#L146 is:

if ann['area'] <= 0 or w < 1 or h < 1:
    continue

If the area property is 0, or does not exist, then this code will skip over the adding of bbox information to gt_bboxes, and the bbox here will not be used. Area is calculated from segmentation information according to MS COCO format guidelines, and has no relation to bounding boxes, and so this code should not be using area to filter for valid bboxes.

If the annotation is bounding boxes only, this code stops that bounding box from being used.

Bug fix I will create a PR for this if you agree that this is a bug and should not behave this way. My proposed solution is to move the first condition of ann['area'] <= 0 to further down in the code to where the mask information is added to gt_masks_ann.

ZwwWayne commented 3 years ago

Hi @waddington-ou-phd-1 , Thanks for your kind suggestion. There are two situations for that. 1. If the area is calculated from the mask, this usually means that we will do instance segmentation with the data, at this time, filtering zero-area mask is necessary. 2. If the area is calculated from bonding boxes, this usually means we are performing detection only tasks. However, at this time we still need to filter out empty boxes to avoid errors.

But you are right, the filtering logic for COCODataset is kind of specialized for COCO dataset and may not be applicable for some customized needs. We indeed have plans to refactoring the data filtering logic in the future, but this may take some time.

waddington-ou-phd-1 commented 3 years ago

Hi @ZwwWayne ,

I have some counter questions as I still think that the area property is only for segmentation masks:

  1. Why would there be an extra property for the area of a box given that this is calculated with a single multiplication of width x height which is already given by the bounding box properties. It is understandable to have this property for segmentation masks as these can be complex shapes of which calculating the area is trickier and longer.
  2. Where does MMDetection use the area property for bounding boxes as provided by the area property of the annotations file?
  3. When checking for validity of bounding boxes why use the area property as well as the sizes of the box? The area is implied by the size so why not only check that the sizes are valid, which MMDetection does already do?
  4. Why would the same property have more than 1 use case, i.e. why is the area property used for masks as well as bounding boxes? If the area of the box is indeed supposed to be in the annotations file, which I argue that it's not supposed to be, wouldn't there be two separate properties, e.g. mask_area and bbox_area? It seems odd that there would be ambiguity around what the property could represent.
  5. If there are segmentation masks present, and therefore the area property is the area of the masks, what happens when MMDetection is doing a bounding box prediction task? You can have segmentation data and bounding box data in the same annotation file, so this is a situation that can be encountered.
waddington-ou-phd-1 commented 3 years ago

Any update on this?

waddington-ou-phd-1 commented 3 years ago

Any update on this?