Refactor filtering mask, fix attribute error

[christian-hiebl]

Motivation

Filtering annotations for a dataset is great (besides the gt_bboxes_ignore option) I changed the implementation of the keep mask and added the attribute error fix in repr as also discussed here #8214 but not included in #8136

Modification

Change of filter annotation masking from appending boolean mask to a list and iterating, to now a bumpy array. Additionally small fix discussed but not included from #8214

BC-breaking (Optional)

I don't think it will break BC.

Use cases (Optional)

Checklist

1. Pre-commit or other linting tools are used to fix the potential lint issues.
2. The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
3. If the modification has potential influence on downstream projects, this PR should be tested with downstream projects, like MMDet or MMCls.
4. The documentation has been modified accordingly, like docstring or example tutorials.

[CLAassistant]

All committers have signed the CLA.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.06%. Comparing base (f8bbba2) to head (e3e696d).
Report is 94 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #8673      +/-   ##
==========================================
- Coverage   64.08%   64.06%   -0.03%     
==========================================
  Files         361      361              
  Lines       29525    29533       +8     
  Branches     5020     5021       +1     
==========================================
- Hits        18922    18921       -1     
- Misses       9589     9592       +3     
- Partials     1014     1020       +6     

Flag	Coverage Δ
unittests	64.06% <100.00%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[christian-hiebl]

@jbwang1997 Do you know why the CUDA build could be failing? Locally I made the changes for this commit with a non CUDA torch and mmcv version. But when the tests are run here, this shouldn't be affected correct?

[jbwang1997]

@jbwang1997 Do you know why the CUDA build could be failing? Locally I made the changes for this commit with a non CUDA torch and mmcv version. But when the tests are run here, this shouldn't be affected correct?

Seem the CI failed when installing the dependence. I have restarted the CI. Let's wait for the results.

[christian-hiebl]

@jbwang1997 Do you know why the CUDA build could be failing? Locally I made the changes for this commit with a non CUDA torch and mmcv version. But when the tests are run here, this shouldn't be affected correct?

Seem the CI failed when installing the dependence. I have restarted the CI. Let's wait for the results.

Ok great, thank you for the restart!

[jbwang1997]

LGTM

[BIGWangYuDong]

gt_bboxes.shape[0] -> instance_num

[BIGWangYuDong]

Is the mask instance_num equal to the box instance_num?

[christian-hiebl]

Yes, I think it should be. And the test cases with both mask instances and gt_bbox instances passes.

[BIGWangYuDong]

Seems that instance_num and keep will over-write if self.by_box and self.by_mask is True.

[BIGWangYuDong]

len(gt_masks) -> instance_num

[BIGWangYuDong]

@christian-hiebl Thanks for your contribution.
I think maybe we can set keep out of the conditions. Such as:

if self.by_box:
    assert 'gt_bboxes' in results
    gt_bboxes = results['gt_bboxes']
    instance_num = gt_bboxes.shape[0]
if self.by_mask:
    assert 'gt_masks' in results
    gt_masks = results['gt_masks']
    instance_num = len(gt_masks)
keep = np.ones(instance_num, dtype=bool)

Not sure if there is a more elegant way to fix the over-write problem.

[christian-hiebl]

@christian-hiebl Thanks for your contribution. I think maybe we can set keep out of the conditions. Such as:

if self.by_box:
    assert 'gt_bboxes' in results
    gt_bboxes = results['gt_bboxes']
    instance_num = gt_bboxes.shape[0]
if self.by_mask:
    assert 'gt_masks' in results
    gt_masks = results['gt_masks']
    instance_num = len(gt_masks)
keep = np.ones(instance_num, dtype=bool)

Not sure if there is a more elegant way to fix the over-write problem.

Thank you for the suggestion! In the init of the FilterAnnotations class there is an assertion, that the filtering is done by either mask or box, so the instance_num won't be over-written.

[BIGWangYuDong]

@christian-hiebl Thanks for your contribution. I think maybe we can set keep out of the conditions. Such as:

if self.by_box:
    assert 'gt_bboxes' in results
    gt_bboxes = results['gt_bboxes']
    instance_num = gt_bboxes.shape[0]
if self.by_mask:
    assert 'gt_masks' in results
    gt_masks = results['gt_masks']
    instance_num = len(gt_masks)
keep = np.ones(instance_num, dtype=bool)

Not sure if there is a more elegant way to fix the over-write problem.

Thank you for the suggestion! In the init of the FilterAnnotations class there is an assertion, that the filtering is done by either mask or box, so the instance_num won't be over-written.

@christian-hiebl

assert by_box or by_mask

if by_box and by_mask all set True, it also works.

[christian-hiebl]

@BIGWangYuDong You are right, 'or' is inclusive so it should also work when both if statements are true. Currently it passes in the test case that a mask and box is given. Also, the instance_num for both is the same, so it will not be a problem.

[BIGWangYuDong]

How about adding an assert before to avoid some potential error?
Such as:

instance_num = None
if self.by_box:
    if instance_num is not None:
        assert instance_num == box_num
if self.by_mask:
    if instance_num is not None:
        assert instance_num == mask_num