Check boxes shape in RoIPool / Align #1968
Conversation
There was a problem hiding this comment.
Thanks a lot for the PR @zhangguanheng66 !
I have a couple of comments.
Also, can you add a test for the check_boxes_is_valid in test_ops.py?
torchvision/ops/roi_align.py
Outdated
| elif isinstance(boxes, torch.Tensor): | ||
| assert boxes.size(1) == 5, 'The boxes tensor shape is not correct as Tensor[K, 5]' | ||
| else: | ||
| assert False, 'boxes shape is not correct' |
There was a problem hiding this comment.
Can we raise a bit more informative error message, like
boxes is expected to be a Tensor[L, 5] or a List[Tensor[K, 4]]
torchvision/ops/roi_align.py
Outdated
| if isinstance(boxes, list): | ||
| for _tensor in boxes: | ||
| assert _tensor.size(1) == 4, \ | ||
| 'The shape of the tensor in the boxes list is not correct as List[Tensor[L, 4]]' | ||
| elif isinstance(boxes, torch.Tensor): | ||
| assert boxes.size(1) == 5, 'The boxes tensor shape is not correct as Tensor[K, 5]' | ||
| else: | ||
| assert False, 'boxes shape is not correct' |
There was a problem hiding this comment.
Looks like this block of code is the same for both roi_align and roi_pool (and we should also add it for ps_roi_align and ps_roi_pool. Can you factor it out into a helper function (maybe called check_boxes_is_valid?) and put it in _utils.py, like we did for convert_boxes_to_roi_format?
|
Thanks for the improvements! Tests are failing, can you have a look? |
|
Most of the test failures are unrelated to this PR (it's a breakage in PyTorch JIT I believe), but there is still one relevant error |
* add checkout/assert in roi_pool * add checkout/assert in roi_align * move check_roi_boxes_shape func to ops/_utils.py * add tests * fix CI * fix CI Co-authored-by: Guanheng Zhang <zhangguanheng@devfair0197.h2.fair>
Summary: * add checkout/assert in roi_pool * add checkout/assert in roi_align * move check_roi_boxes_shape func to ops/_utils.py * add tests * fix CI * fix CI Pull Request resolved: #2429 Reviewed By: zhangguanheng66 Differential Revision: D22437763 Pulled By: fmassa fbshipit-source-id: 78727f3bfe2514e2c193e2b27d9146693fa800b0 Co-authored-by: Guanheng Zhang <zhangguanheng@devfair0197.h2.fair>
Fix #1957
The boxes should be Tensor[K, 5] or List[Tensor[L, 4]]