Maskrcnn-benchmark: TO_REMOVE=1 at different places?

[sadanand-singh]

❓ Questions and Help

What is TO_REMOVE=1 variable at many places in the code? There is a comment regarding it, "TODO remove". Can someonple please elaborate on this. thanks.

[fmassa]

Hi,

This is legacy behavior from Detectron, which exists because back then bounding boxes would start from 1 and indexing was inclusive in matlab on the last item, so the width of a bounding box would be calculated as x2 - x1 + 1.
Nowadays, this could probably be removed, but this would be a backwards-incompatible change and was left here to maintain compatibility with Detectron models.

But if we don't want to keep compatibility with Detectron models, the TO_REMOVE can be removed.

In our case, we will stick with it for backward-compatibility reasons.

[gathierry]

@fmassa Thanks for the explanation.
But in BoxCoder, if width and height are computed with TO_REMOVE, the center location should be ex_ctr_x = proposals[:, 0] + 0.5 * (ex_widths - TO_REMOVE) right? There's no - TO_REMOVE in current code

[fmassa]

The implementation is BoxCoder was taken exactly as is from Detectron, which uses the +1, so we are consistent with it.

I'm ccing @rbgirshick as the one who originally implemented it. There might be a subtle problem in https://github.com/facebookresearch/maskrcnn-benchmark/blob/e60f4ec8dc50531debcfd5ae671ea167b5b7a1d9/maskrcnn_benchmark/modeling/box_coder.py#L35 , but I'm not sure if it really makes a difference

[rbgirshick]

One should write a unit test like this to verify that the "encoding" and "decoding" produce the expected result.

[gathierry]

I agree. This seems won't make great differences. Just found it incoherent when generating_anchors
https://github.com/facebookresearch/maskrcnn-benchmark/blob/e60f4ec8dc50531debcfd5ae671ea167b5b7a1d9/maskrcnn_benchmark/modeling/rpn/anchor_generator.py#L250

[JoyHuYY1412]

Hi,

This is legacy behavior from Detectron, which exists because back then bounding boxes would start from 1 and indexing was inclusive in matlab on the last item, so the width of a bounding box would be calculated as x2 - x1 + 1.
Nowadays, this could probably be removed, but this would be a backwards-incompatible change and was left here to maintain compatibility with Detectron models.

But if we don't want to keep compatibility with Detectron models, the TO_REMOVE can be removed.

In our case, we will stick with it for backward-compatibility reasons.

Hi, according to your explanation, I think the condition to remove boxes here should be '>=' instead of '>' ? Since if x2=x1, the box is also valid.
https://github.com/facebookresearch/maskrcnn-benchmark/blob/24c8c90efdb7cc51381af5ce0205b23567c3cd21/maskrcnn_benchmark/structures/bounding_box.py#L221-L222