Eliminate runtime cyclic dependencies

[datumbox]

A previous approach of assigning _F on constructor faced issues on the data loader test as the the unit-test hang. This PR tries an alternative approach of making _F a property, doing a lazy import and caching the the module reference on the class level. This seems to avoid the previous test issue.

[datumbox]

Lazy import the module the first time we need it. It's shared across all _Feature subclasses.

[pmeier]

Given that there were problems with the DataLoader before, here are the tests that we use for the prototype datasets:

vision/test/test_prototype_builtin_datasets.py

Lines 104 to 135 in b4b246a

	@pytest.mark.parametrize("only_datapipe", [False, True])
	@parametrize_dataset_mocks(DATASET_MOCKS)
	def test_traversable(self, dataset_mock, config, only_datapipe):
	dataset, _ = dataset_mock.load(config)
	traverse(dataset, only_datapipe=only_datapipe)
	@parametrize_dataset_mocks(DATASET_MOCKS)
	def test_serializable(self, dataset_mock, config):
	dataset, _ = dataset_mock.load(config)
	pickle.dumps(dataset)
	# This has to be a proper function, since lambda's or local functions
	# cannot be pickled, but this is a requirement for the DataLoader with
	# multiprocessing, i.e. num_workers > 0
	def _collate_fn(self, batch):
	return batch
	@pytest.mark.parametrize("num_workers", [0, 1])
	@parametrize_dataset_mocks(DATASET_MOCKS)
	def test_data_loader(self, dataset_mock, config, num_workers):
	dataset, _ = dataset_mock.load(config)
	dl = DataLoader(
	dataset,
	batch_size=2,
	num_workers=num_workers,
	collate_fn=self._collate_fn,
	)
	next(iter(dl))

Here is a version to run the same tests for different transforms:

import pickle

import pytest
import torch

from torch.utils.data import DataLoader
from torch.utils.data.graph import traverse
from torchdata.datapipes.iter import IterableWrapper
from torchvision.prototype import transforms, features

# TODO: fill me up for more comprehensive tests!
TRANSFORMS = [
    transforms.Resize((8, 8)),
]

transforms = pytest.mark.parametrize("transform", TRANSFORMS, ids=lambda transform: type(transform).__name__)


@pytest.mark.parametrize("only_datapipe", [False, True])
@transforms
def test_traversable(transform, only_datapipe):
    dp = IterableWrapper([]).map(transform)

    traverse(dp, only_datapipe=only_datapipe)


@transforms
def test_serializable(transform):
    dp = IterableWrapper([]).map(transform)

    pickle.dumps(dp)


def _collate_fn(batch):
    return batch


@pytest.mark.parametrize("num_workers", [0, 1])
@transforms
def test_data_loader(transform, num_workers):
    dp = IterableWrapper([features.Image(torch.rand(3, 16, 16)) for _ in range(5)], deepcopy=False).map(transform)

    dl = DataLoader(
        dp,
        batch_size=2,
        num_workers=num_workers,
        collate_fn=_collate_fn,
    )

    list(dl)

So far the PR seems good.

[datumbox]

@pmeier Thanks for the suggestion for additional tests.

I know that these tests take a while due to the large number of datasets. Shall I add just one transform for now to enhance them? aka

    def test_data_loader(self, dataset_mock, config, num_workers):
        dataset = dataset_mock.load(config)[0].map(transforms.Resize((8, 8)))

        dl = DataLoader(
            dataset,
            batch_size=2,
            num_workers=num_workers,
            collate_fn=self._collate_fn,
        )

        next(iter(dl))

I was thinking of doing this for all 3 tests you quoted above.

[pmeier]

You need at least two transforms

transforms.Compose(
    [
        transforms.DecodeImage(),
        transforms.Resize(8, 8),
    ]
)

Otherwise, you will have no images inside the sample.

I'm ok with adding these there given that our current transforms compatibility tests are quite "minimal":

vision/test/test_prototype_builtin_datasets.py

Lines 98 to 102 in b4b246a

	@parametrize_dataset_mocks(DATASET_MOCKS)
	def test_transformable(self, dataset_mock, config):
	dataset, _ = dataset_mock.load(config)
	next(iter(dataset.map(transforms.Identity())))

@NicolasHug do you have an opinion here?

[pmeier]

I'm fine with the changes to the datasets tests minus some small nits. There is test_transformable which is outside of the scope that GH let's me comment. It will be obsolete after your changes and you can delete it.

Given that only the test_data_loader actually executes the transform, I don't think we should see a significant slow down. Still, let's wait for CI to complete to compare the timings. @NicolasHug any objections?

[datumbox]

@pmeier I'll remove the extra tests to give time to you and @NicolasHug to finalize the strategy. They are not strictly needed for this work.

BTW I already see some unrelated failures related to the resize on bboxes that might be worth looking into.

[pmeier]

Apart from my paranoia in #6476 (comment), I think this is GTG.

[datumbox]

Merging, the failing tests are unrelated and are caused by dependencies issues from core.