use non-random images for interpolation kernels for testing #6977
Conversation
| @@ -69,6 +70,7 @@ def compare(self) -> None: | |||
| self._compare_attributes(actual, expected) | |||
|
|
|||
| actual, expected = self._equalize_attributes(actual, expected) | |||
| actual, expected = self._promote_for_comparison(actual, expected) | |||
There was a problem hiding this comment.
This was a bug. Without this promotion, we are later subtracting uint8 values for the absolute difference. They wrap around:
>>> a = torch.tensor(10, dtype=torch.uint8)
>>> b = torch.tensor(11, dtype=torch.uint8)
>>> a - b
tensor(255, dtype=torch.uint8)
>>> abs(a - b)
tensor(255, dtype=torch.uint8)If we convert to a singed integer dtype first (here int64) we get the correct absolute diff:
>>> a = torch.tensor(10, dtype=torch.int64)
>>> b = torch.tensor(11, dtype=torch.int64)
>>> a - b
tensor(-1, dtype=torch.int64)
>>> abs(a - b)
tensor(1, dtype=torch.int64)This bug only affected the comparisons that used agg_method=....
| @@ -319,17 +320,6 @@ def sample_inputs_resize_mask(): | |||
| yield ArgsKwargs(mask_loader, size=[min(mask_loader.shape[-2:]) + 1]) | |||
|
|
|||
|
|
|||
| @pil_reference_wrapper | |||
| def reference_resize_mask(*args, **kwargs): | |||
There was a problem hiding this comment.
Since all of our mask kernels are just wrapping the image ones, there is no need to compare both against a reference. Checking the image kernel is sufficient.
|
|
||
|
|
||
| def make_image_loaders_for_interpolation( | ||
| sizes=((233, 147),), |
There was a problem hiding this comment.
I didn't use the default shapes here since they are pretty small and thus the area of the border of the ellipse is quite large compared to the overall image and thus larger tolerances would need to be larger. Open for discussion about this.
| closeness_kwargs=float32_vs_uint8_pixel_difference(60, agg_method="mean"), | ||
| closeness_kwargs={ | ||
| **float32_vs_uint8_pixel_difference(6, agg_method="mean"), | ||
| **cuda_vs_cpu_pixel_difference(), |
There was a problem hiding this comment.
Driveby that seems to have snuck in since the last time I was testing against CUDA.
| ), | ||
| KernelInfo( | ||
| F.elastic_video, | ||
| sample_inputs_fn=sample_inputs_elastic_video, | ||
| closeness_kwargs=cuda_vs_cpu_pixel_difference(), |
There was a problem hiding this comment.
LGTM. IMO the proposed approach is the better alternative from the other 4 described at #6937 (comment)
…6977) Summary: * use non-random images for interpolation kernels for testing * use real image rather than artificial * cleanup Reviewed By: YosuaMichael Differential Revision: D41648540 fbshipit-source-id: d3685e4ade1e6729c64e9ec6002d6914ff070a52
Fixes #6937 by using option 1. proposed in #6937 (comment).
cc @vfdev-5 @datumbox @bjuncek