Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Remove Detectron dependency #457

Merged
merged 3 commits into from
Feb 18, 2019
Merged

Remove Detectron dependency #457

merged 3 commits into from
Feb 18, 2019

Conversation

botcs
Copy link
Contributor

@botcs botcs commented Feb 18, 2019

Contribution to CityScapes -> COCO converter, following up on #447 .

I have looked into the boxes.py to swap these lines:

import detectron.utils.cython_bbox as cython_bbox
import detectron.utils.cython_nms as cython_nms
from maskrcnn_benchmark.structures.boxlist_ops import boxlist_iou
from maskrcnn_benchmark.structures.boxlist_ops import boxlist_nms

However some functions are missing from the boxlist_ops like the soft_nms .

So I just tried to modify the maskrcnn-benchmark/tools/cityscapes/convert_cityscapes_to_coco.py script.
Here we have polys_to_boxes function from segms.py and I could not find its analogous in the maskrcnn_benchmark lib. It seems to me that the original function in segms.py is using pure lists, while for the BoxList.convert I had to create a new entity for each object.

Seeing that the whole box is finally converted back to native python list seemed to have huge overhead, so I just wrote two auxiliary functions reusing the boxList's convert method( https://github.com/facebookresearch/maskrcnn-benchmark/blob/master/maskrcnn_benchmark/structures/bounding_box.py#L67L70 )
and Detectron's polys_to_boxes ( https://github.com/facebookresearch/Detectron/blob/b5dcc0fe1d091cb70f9243939258215dd63e3dfa/detectron/utils/segms.py#L135L140 ) to be applicable to a single instance:

def poly_to_box(poly):
    """Convert a polygon into a tight bounding box."""
    x0 = min(min(p[::2]) for p in poly)
    x1 = max(max(p[::2]) for p in poly)
    y0 = min(min(p[1::2]) for p in poly)
    y1 = max(max(p[1::2]) for p in poly)
    box_from_poly = [x0, y0, x1, y1]

    return box_from_poly

def xyxy_to_xywh(xyxy_box):
    xmin, ymin, xmax, ymax = xyxy_box
    TO_REMOVE = 1
    xywh_box = (xmin, ymin, xmax - xmin + TO_REMOVE, ymax - ymin + TO_REMOVE)
    return xywh_box

The upside of this solution that it uses native python lists so there is less overhead (if that counts here).

I haved tested it in a fresh conda environment using only the maskrcnn_benchmark install and it has been producing the same output as the previous.

I have looked into the boxes.py to swap [these lines](https://github.com/facebookresearch/Detectron/blob/8170b25b425967f8f1c7d715bea3c5b8d9536cd8/detectron/utils/boxes.py#L51L52):
```
import detectron.utils.cython_bbox as cython_bbox
import detectron.utils.cython_nms as cython_nms
```

```
from maskrcnn_benchmark.structures.boxlist_ops import boxlist_iou
from maskrcnn_benchmark.structures.boxlist_ops import boxlist_nms
```
However some functions are missing from the `boxlist_ops` like the [`soft_nms`](https://github.com/facebookresearch/Detectron/blob/master/detectron/utils/cython_nms.pyx#L98L203) .

So I just tried to modify the `maskrcnn-benchmark/tools/cityscapes/convert_cityscapes_to_coco.py` script.
Here we have `polys_to_boxes` function from `segms.py` and I could not find its analogous in the maskrcnn_benchmark lib. It seems to me that the original function in `segms.py` is using pure lists so I just wrote two auxiliary functions reusing the boxList's convert method( https://github.com/facebookresearch/maskrcnn-benchmark/blob/master/maskrcnn_benchmark/structures/bounding_box.py#L67L70 )
and Detectron's polys_to_boxes ( https://github.com/facebookresearch/Detectron/blob/b5dcc0fe1d091cb70f9243939258215dd63e3dfa/detectron/utils/segms.py#L135L140 ):
```
def poly_to_box(poly):
    """Convert a polygon into a tight bounding box."""
    x0 = min(min(p[::2]) for p in poly)
    x1 = max(max(p[::2]) for p in poly)
    y0 = min(min(p[1::2]) for p in poly)
    y1 = max(max(p[1::2]) for p in poly)
    box_from_poly = [x0, y0, x1, y1]

    return box_from_poly

def xyxy_to_xywh(xyxy_box):
    xmin, ymin, xmax, ymax = xyxy_box
    TO_REMOVE = 1
    xywh_box = (xmin, ymin, xmax - xmin + TO_REMOVE, ymax - ymin + TO_REMOVE)
    return xywh_box
```
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 18, 2019
Copy link
Contributor

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick PR!

Can you also remove the mention of detectron in the README, now that it's not a dependency anymore?

@@ -136,13 +150,14 @@ def convert_cityscapes_instance_only(
'motorcycle',
'bicycle',
]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you remove the whitespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

for data_set, ann_dir in zip(sets, ann_dirs):
print('Starting %s' % data_set)
ann_dict = {}
images = []
annotations = []
ann_dir = os.path.join(data_dir, ann_dir)
print('ann_dir: %15s'%ann_dir)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do we need this print?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found it useful to keep track of this, but not necessary.

How about adding tqdm progressbar? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not, but it would add an extra dependency :-)
Maybe a print from time to time would be enough

Copy link
Contributor

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks a lot!

@fmassa fmassa merged commit 0f9476b into facebookresearch:master Feb 18, 2019
@botcs
Copy link
Contributor Author

botcs commented Feb 18, 2019

Thanks for the quick PR!

Can you also remove the mention of detectron in the README, now that it's not a dependency anymore?

Sure, forgot to do in this PR, but filed another...

@botcs botcs deleted the patch-3 branch April 1, 2019 12:36
Lyears pushed a commit to Lyears/maskrcnn-benchmark that referenced this pull request Jun 28, 2020
* Remove Detectron dependency

I have looked into the boxes.py to swap [these lines](https://github.com/facebookresearch/Detectron/blob/8170b25b425967f8f1c7d715bea3c5b8d9536cd8/detectron/utils/boxes.py#L51L52):
```
import detectron.utils.cython_bbox as cython_bbox
import detectron.utils.cython_nms as cython_nms
```

```
from maskrcnn_benchmark.structures.boxlist_ops import boxlist_iou
from maskrcnn_benchmark.structures.boxlist_ops import boxlist_nms
```
However some functions are missing from the `boxlist_ops` like the [`soft_nms`](https://github.com/facebookresearch/Detectron/blob/master/detectron/utils/cython_nms.pyx#L98L203) .

So I just tried to modify the `maskrcnn-benchmark/tools/cityscapes/convert_cityscapes_to_coco.py` script.
Here we have `polys_to_boxes` function from `segms.py` and I could not find its analogous in the maskrcnn_benchmark lib. It seems to me that the original function in `segms.py` is using pure lists so I just wrote two auxiliary functions reusing the boxList's convert method( https://github.com/facebookresearch/maskrcnn-benchmark/blob/master/maskrcnn_benchmark/structures/bounding_box.py#L67L70 )
and Detectron's polys_to_boxes ( https://github.com/facebookresearch/Detectron/blob/b5dcc0fe1d091cb70f9243939258215dd63e3dfa/detectron/utils/segms.py#L135L140 ):
```
def poly_to_box(poly):
    """Convert a polygon into a tight bounding box."""
    x0 = min(min(p[::2]) for p in poly)
    x1 = max(max(p[::2]) for p in poly)
    y0 = min(min(p[1::2]) for p in poly)
    y1 = max(max(p[1::2]) for p in poly)
    box_from_poly = [x0, y0, x1, y1]

    return box_from_poly

def xyxy_to_xywh(xyxy_box):
    xmin, ymin, xmax, ymax = xyxy_box
    TO_REMOVE = 1
    xywh_box = (xmin, ymin, xmax - xmin + TO_REMOVE, ymax - ymin + TO_REMOVE)
    return xywh_box
```

* removed leftovers

* Update convert_cityscapes_to_coco.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants