-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Support for ROI Pooling #592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks very nice, thanks!
I particularly liked that you added support for non-contiguous grad_output, that was a bug in the previous implementation.
Could you address the few comments I've added? Otherwise looks good to merge!
@@ -193,7 +197,11 @@ at::Tensor ROIPool_backward_cuda(const at::Tensor& grad, | |||
pooled_height, | |||
pooled_width, | |||
grad_input.data<scalar_t>(), | |||
rois.data<scalar_t>()); | |||
rois.data<scalar_t>(), | |||
n_stride, |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
offset_bottom_diff + argmax, | ||
static_cast<T>(offset_top_diff[ph * pooled_width + pw])); | ||
|
||
const int pooled_height, const int pooled_width, T* bottom_data, |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
@@ -30,7 +28,7 @@ __global__ void RoIPoolFForward(const int nthreads, const T* bottom_data, | |||
int roi_end_w = round(offset_bottom_rois[3] * spatial_scale); | |||
int roi_end_h = round(offset_bottom_rois[4] * spatial_scale); | |||
|
|||
// Force malformed ROIs to be 1x1 | |||
// Force malformed ROIs to be 1x1 or HxW |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
const int n_stride, const int c_stride, | ||
const int h_stride, const int w_stride) { | ||
|
||
CUDA_1D_KERNEL_LOOP(index, nthreads) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
This reverts commit ede93b2.
@fmassa @varunagrawal I'm unable to compile the latest revision of
Could you please give me some directions about which changes must be done in order to compile this branch without any errors? Thanks |
@andfoy thanks for reporting this. This is an issue in ROIAlign and not ROIPool. This PR is specific to ROIPool. I am still working on fixing this. Thank you for your patience. I'll get back to you on fixes for this really soon. |
@varunagrawal Thanks for your reply, is this error related to: pytorch/pytorch#12360? |
Yep. That is exactly it. |
@varunagrawal Do you think that if all tensor creation operations are replaced by the new API, RoIAlign/Pool would work? |
Thanks for updating the layers branch @varunagrawal |
@fmassa can you please elaborate on the last comment? Will the layers be a part of pytorch proper or a part of torchvision? More importantly, will this be just a list of approved layers or an actual release of code? |
@varunagrawal my comment was not very clear, sorry. I meant we will be releasing an object detection library, which will contain the ROI layers. The layers will initially be part of this library. They might be moved out into torchvision at a later moment, but not initially |
Ah gotcha. I did talk to @soumith about this in 2017, so feel free to rope me in. |
* ROI Pooling with tests. Fix for cuda context in ROI Align. * renamed bottom and top to follow torch conventions
* ROI Pooling with tests. Fix for cuda context in ROI Align. * renamed bottom and top to follow torch conventions
* ROI Pooling with tests. Fix for cuda context in ROI Align. * renamed bottom and top to follow torch conventions
* ROI Pooling with tests. Fix for cuda context in ROI Align. * renamed bottom and top to follow torch conventions
* ROI Pooling with tests. Fix for cuda context in ROI Align. * renamed bottom and top to follow torch conventions
* Initial layout for layers with cpp extensions * Move files around * Fix import after move * Add support for multiple types to ROIAlign * Different organization CUDA extensions work now * Cleanups * Reduce memory requirements for backwards * Replace runtime_error by AT_ERROR * Add nms test * Add support for compilation using CPP extensions * Change folder structure * Add ROIPool cuda * Cleanups * Add roi_pool.py * Fix lint * Add initial structures folder for bounding boxes * Assertion macros compatible with pytorch master (#540) * Support for ROI Pooling (#592) * ROI Pooling with tests. Fix for cuda context in ROI Align. * renamed bottom and top to follow torch conventions * remove .type().tensor() calls in favor of the new approach to tensor initialization (#626) * Consistent naming for rois variable (#627) * remove .type().tensor() calls in favor of the new approach to tensor initialization * Consistent naming for rois variable in ROIPool * ROIPool: Support for all datatypes (#632) * Use of torch7 naming scheme for ROIAlign forward and backward * use common cuda helpers in ROIAlign * use .options() in favor of .type() where applicable * Added tests for forward pass of ROIAlign, as well as more consistent naming scheme for CPU vs CUDA * working ROIAlign cuda backwards pass * working ROIAlign backwards pass for CPU * added relevant headers for ROIAlign backwards * tests for ROIAlign layer * replace .type() with .options() for tensor initialization in ROIAlign layers * support for Half types in ROIAlign * gradcheck tests for ROIAlign * updated ROIPool on CPU to work with all datatypes * updated and cleaned tests for ROI Pooling * Fix rebase problem * Remove structures folder * Improve cleanup and bugfix in test_layers * Update C++ headers * Add CUDAGuard to cu files * Add more checks to layers * Add CUDA NMS and tests * Add multi-type support for NMS CUDA * Avoid using THCudaMalloc * Add clang-format and reformat c++ code * Remove THC includes * Rename layers to ops * Add documentation and rename functions * Improve the documentation a bit * Fix some lint errors * Fix remaining lint inssues * Area computation doesn't add +1 in NMS * Update CI to use PyTorch nightly * Make NMS return indices sorted according to the score * Address reviewer comments * Lint fixes * Improve doc for roi_align and roi_pool * move to xenial * Fix bug pointed by @lopuhin * Fix RoIPool reference implementation in Python 2 Also fixes a bug in the clip_boxes_to_image -- this function needs a test! * Remove change in .travis
ROI Pooling with tests. Fix for cuda context in ROI Align.
@fmassa please can you review this?