[ONNX] Export ROIAlign with aligned=True

[neginraoof]

[ONNX] Export ROIAlign with aligned=True

[fmassa]

Thanks for the PR!

I think there are still a few issues that need to be addressed, let me know what you think.

Also, the ONNX CI is currently broken, and would be great to have it fixed before we can merge this.

[fmassa]

This is unfortunately not all that needs to be changed so that the implementation supports the aligned flag.

Indeed, this only works if the spatial_scale is 1, and if the boxes are not malformed.

See

vision/torchvision/csrc/cpu/ROIAlign_cpu.cpp

Lines 138 to 150 in 3e0f5a6

	T offset = aligned ? (T)0.5 : (T)0.0;
	T roi_start_w = offset_rois[1] * spatial_scale - offset;
	T roi_start_h = offset_rois[2] * spatial_scale - offset;
	T roi_end_w = offset_rois[3] * spatial_scale - offset;
	T roi_end_h = offset_rois[4] * spatial_scale - offset;
	T roi_width = roi_end_w - roi_start_w;
	T roi_height = roi_end_h - roi_start_h;
	if (!aligned) {
	// Force malformed ROIs to be 1x1
	roi_width = std::max(roi_width, (T)1.);
	roi_height = std::max(roi_height, (T)1.);
	}

for the locations where the aligned flag plays a role. I believe we would need at least to do something like

if aligned:
    rois = (rois * spatial_scale - 0.5) / spatial_scale
    # or equivalently
    rois = rois - 0.5 / spatial_scale

but the difference wrt the clamping in

vision/torchvision/csrc/cpu/ROIAlign_cpu.cpp

Lines 146 to 150 in 3e0f5a6

	if (!aligned) {
	// Force malformed ROIs to be 1x1
	roi_width = std::max(roi_width, (T)1.);
	roi_height = std::max(roi_height, (T)1.);
	}

persists

[neginraoof]

Thanks a lot for reviewing this. It was very helpful.
Since ONNX forces all ROIs to be 1x1 or larger, I think fixing the export for malformed ROIs needs a change in spec.

[fmassa]

Can you test for more configurations of the roi (including malformed boxes), and different scale_factor?

[fmassa]

LGTM, thanks!