🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/9277

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 3eb5cb6 with merge base dccf466 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@zy1git What is the strategy for creating the tests for the transforms with CV-CUDA backends? Do we want to have all the tests live entirely inside the existing classes or make a new class?

The PRs for gaussian_blur, normalize, and to_dtype I made all use new classes, but I can switch it be more centralized.

@justincdavis replying to your question in #9277 (comment): we prefer centralizing the tests in the existing test class. The idea is that, as much as possible, we'd just add CV-CUDA as a parametrization entry with pytest.mark.parametrize to the existing tests. Antoine and I left a few comments related to that above. Does that make sense?

@NicolasHug Makes sense! I will follow the comments you and Antoine left on this PR

@AntoineSimoulin @NicolasHug @zy1git

I would propose that we take an approach like this to simplify the comparison of CV-CUDA tensors with PIL references.

We would add the following function to common_utils.py

def cvcuda_to_pil_compatible_tensor(tensor):
    tensor = cvcuda_to_tensor(tensor)
    if tensor.ndim != 4:
        raise ValueError(f"CV-CUDA Tensor should be 4 dimensional. Got {tensor.ndim} dimensions.")
    if tensor.shape[0] != 1:
        raise ValueError(f"CV-CUDA Tensor should have batch dimension 1 for comparison with PIL.Image.Image. Got {tensor.shape[0]}.")
    return tensor.squeeze(0).cpu()

Then we would modify ImagePair as follows

class ImagePair(TensorLikePair):
    def __init__(
        self,
        actual,
        expected,
        *,
        mae=False,
        **other_parameters,
    ):
        if all(isinstance(input, PIL.Image.Image) for input in [actual, expected]):
            actual, expected = (to_image(input) for input in [actual, expected])
        elif CVCUDA_AVAILABLE and all(isinstance(input, _import_cvcuda().Tensor) for input in [actual, expected]):
            actual, expected = (cvcuda_to_tensor(input) for input in [actual, expected])
        elif CVCUDA_AVAILABLE and isinstance(actual, _import_cvcuda().Tensor) and isinstance(expected, PIL.Image.Image):
            actual = cvcuda_to_pil_compatible_tensor(actual)
            expected = to_image(expected)
        elif CVCUDA_AVAILABLE and isinstance(actual, _import_cvcuda().Tensor):
            actual = cvcuda_to_pil_compatible_tensor(actual)

        super().__init__(actual, expected, **other_parameters)
        self.mae = mae

Then when we compare the actual tensor (cvcuda.Tensor) to the expected (PIL.Image.Image), the conversion will be handled automatically for us.

Additionally, with the helper function cvcuda_to_pil_compatible_tensor, we can simplify the logic for handling CV-CUDA specific comparisions. For example, in TestRgbToGrayscale::test_image_correctness, the logic for handling CV-CUDA goes from:

if make_input is make_image_cvcuda:
        actual = F.cvcuda_to_tensor(actual).to(device="cpu")
        actual = actual.squeeze(0)
        # drop the batch dimension
        image = F.cvcuda_to_tensor(image).to(device="cpu")
        image = image.squeeze(0)

to

# here the conversion of actual is handled in either assert_close or assert_equal itself
if make_input is make_image_cvcuda:
        image = cvcuda_to_pil_compatible_tensor(image)

These changes are integrated in this PR

is it feature still in prototype state? can we also add other transforms like resizing others?
i couldnt find any issue about the cv-cuda backend features so maybe im missing something or if its internal
thank you.

@abhi-glitchhg This will likely be released as Beta. Either in the next version in late Jan, or the following one. We do plan to add more transforms support, we just need to make sure that the output results are the same with our current tensor GPU backend.

can i help implementing some transforms? cc @justincdavis as you are handling most of the prs regarding cv-cuda.

Thank you so much for offering to help @abhi-glitchhg ! We would have loved to make this a community-driven project, like the various ones you participated in in the past, but for this one we're directly partnering with the CV-CUDA folks from NVidia. @justincdavis has already implemented almost all transforms (see current PRs + many in the backlog), and what's left is for us to review the PRs.

I'll definitely let you know in the future if there are any interesting projects that could be in scope for the community. Thanks again!

Hey @NicolasHug!

You merged this PR, but no labels were added.
The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

So looking at the logs of the cvcuda job (e.g. from the hud) you'll see that some of the new cvcuda tests are being skipped.

The reason is: they are missing the needs_cuda pytest mark.

We have a mechanism in place (link) which:

• skips the needs_cuda tests on CPU CI instances
• skips the cpu tests on GPU CI instances.

The new cvcuda CI job is a GPU CI instance. It skips the CPU tests. But since some of the CV-CUDA tests aren't marked with needs_cuda they are considered to be CPU tests and they are then skipped.

Some of the newly added tests here implicitly have the needs_cuda mark because they rely on the

 @pytest.mark.parametrize("device", cpu_and_cuda())

parametrization, which will be adding the needs_cuda mark when needed. But for the tests that do not use that, they're missing the mark and we need to add it.

I think what we can do is, for the tests that are currently using

marks=pytest.mark.skipif(not CVCUDA_AVAILABLE, reason="CVCUDA is not available"),

we should do something like:

CVCUDA_MARK = (pytest.mark.skipif(not CVCUDA_AVAILABLE, reason="CVCUDA is not available"), needs_cuda)
marks=CVCUDA_MARK

(not sure it'll work as-is, but you get the idea.

I hope that makes sense, let's chat offline if needed.