Add option to prune bboxes based on % area in Crop ROI

[5had3z]

Category:

New feature

Description:

Following on #5366, I made some modifications to RandomBBoxCropImpl to enable pruning bboxes based on their remaining area within the ROI, while leaving the option to use the original centroid algorithm (which is currently default). This simply finds the intersection between the crop and bbox and divides this by the bbox area. The bbox is kept if this result is above a threshold.

Some results of this change are also shown in the aforementioned thread.

Additional information:

Affected modules and functionalities:

Added new argument to random_bbox_crop, namely bbox_prune, which is a float that signals to use the old algorithm (by default), or the new algorithm.

Key points relevant for the review:

Naming things is a difficult part of CS, I just copied my original "one new parameter" implementation that has overloaded functionality. We can change it to a string to signal which algoirthm, and add another parameter for the threshold tolerance. Always better to have more than one opinion on API design.

Tests:

I intend to add some more tests to operator_2/test_random_bbox_crop.py.

• Existing tests apply
• New tests added
  • Python tests
  • GTests
  • Benchmark
  • Other
• N/A

Checklist

Documentation

• Existing documentation applies
• Documentation updated
  • Docstring
  • Doxygen
  • RST
  • Jupyter
  • Other
• N/A

DALI team only

Requirements

• Implements new requirements
• Affects existing requirements
• N/A

REQ IDs: N/A

JIRA TASK: N/A

[JanuszL]

How about instead of using -1 as an off value, call ArgumentDefined to learn if the argument was used at all?

[JanuszL]

Hi @5had3z,

Thank you for your contribution. Let us check it and share our comments with you soon.

[jantonguirao]

Hi @5had3z. Thanks a lot for your contribution. I took the liberty to propose some changes to the API, which I put in a PR to this branch: 5had3z#1

The change is to separate the two filtering methods with bbox_prune_by_centroid and bbox_prune_area_threshold so that they can be applied independently and avoid having magic values (-1).

[5had3z]

Hi @5had3z. Thanks a lot for your contribution. I took the liberty to propose some changes to the API, which I put in a PR to this branch: 5had3z#1

The change is to separate the two filtering methods with bbox_prune_by_centroid and bbox_prune_area_threshold so that they can be applied independently and avoid having magic values (-1).

Hi @jantonguirao, I prefer @JanuszL's suggestion for testing the presence of a bbox_prune_area_threshold argument specified by the user to switch between algorithms. With your current implementation, the user has to explicitly set bbox_prune_by_centroid=False to only get the by-area branch. The bbox_prune_area_threshold_ > 0.0f branch condition also ommits the inherent option of no pruning of any boxes with some presence in the ROI (setting this as epsilon will also do the job, but setting this as "greater than zero" is simpler). I don't think it makes sense run both algorithms at the same time anyway.

Two Args

# No args, Prune by centroid - OK
outputs= fn.random_bbox_crop(*args)
# Specify threshold, dominated by centroid pruning - OOPS
outputs = fn.random_bbox_crop(*args, bbox_prune_area_threshold=0.1)
# Both Args, Prune by area - OK
outputs = fn.random_bbox_crop(*args, bbox_prune_area_threshold=0.1, bbox_prune_by_centroid=False)

Testing for Presence of area

# No args, Prune by centroid - OK
outputs= fn.random_bbox_crop(*args)
# Specify threshold, Prune by area - OK
outputs = fn.random_bbox_crop(*args, bbox_prune_area_threshold=0.1)

[jantonguirao]

Hi @5had3z. Thanks a lot for your contribution. I took the liberty to propose some changes to the API, which I put in a PR to this branch: 5had3z#1
The change is to separate the two filtering methods with bbox_prune_by_centroid and bbox_prune_area_threshold so that they can be applied independently and avoid having magic values (-1).

Hi @jantonguirao, I prefer @JanuszL's suggestion for testing the presence of a bbox_prune_area_threshold argument specified by the user to switch between algorithms. With your current implementation, the user has to explicitly set bbox_prune_by_centroid=False to only get the by-area branch. The bbox_prune_area_threshold_ > 0.0f branch condition also ommits the inherent option of no pruning of any boxes with some presence in the ROI (setting this as epsilon will also do the job, but setting this as "greater than zero" is simpler). I don't think it makes sense run both algorithms at the same time anyway.

Two Args

# No args, Prune by centroid - OK
outputs= fn.random_bbox_crop(*args)
# Specify threshold, dominated by centroid pruning - OOPS
outputs = fn.random_bbox_crop(*args, bbox_prune_area_threshold=0.1)
# Both Args, Prune by area - OK
outputs = fn.random_bbox_crop(*args, bbox_prune_area_threshold=0.1, bbox_prune_by_centroid=False)

Testing for Presence of area

# No args, Prune by centroid - OK
outputs= fn.random_bbox_crop(*args)
# Specify threshold, Prune by area - OK
outputs = fn.random_bbox_crop(*args, bbox_prune_area_threshold=0.1)

Sure, that works as well. Let's just avoid the magic value for the default behaviour and explicitly check if the argument was provided or not.

[5had3z]

Ironically the "meta-test" of my python intersection function was the problem that had me the most stuck. Accidentially had any(shape) < 0 instead of any(shape < 0) resulting in double -ve sides being reported as positive area, and the initial "zero" intersection tests were boxes touching corners.

[JanuszL]

Suggested change

	if intersec / box_area > thresh:
	if intersec / box_area >= thresh:

I guess the tests need to be updated as well.

[jantonguirao]

!build

[dali-automaton]

CI MESSAGE: [13523046]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [13523046]: BUILD PASSED