Skip slow sampler tests

[adamjstewart]

The parallel data loader sampler tests are extremely slow on macOS/Windows. See #102 for details. This PR skips them for normal test usage, but keeps them for longer integration testing. Even without these tests we still have 100% test coverage for the samplers.

Before

$ time pytest tests/samplers
...
real	7m30.759s
user	2m18.260s
sys	0m52.247s

After

$ time pytest tests/samplers
...
real	0m4.402s
user	0m3.223s
sys	0m1.057s

[calebrob6]

If I understand this correctly, our dataloaders are extremely slow on Windows and macOS?

If so, this seems like a really big issue that would make torchgeo practically worthless in these cases. Am I missing something?

[adamjstewart]

See https://discuss.pytorch.org/t/data-loader-multiprocessing-slow-on-macos/131204, this is an issue for all of PyTorch on macOS/Windows, it has nothing to do with TorchGeo.

Although our DataLoader are also useless on macOS/Windows, see #184. That's something we can do something about, I don't think we can do anything about the speed issues. There's a lot of overhead if you have to pickle the entire environment.

[calebrob6]

https://discuss.pytorch.org/t/data-loader-multiprocessing-slow-on-macos/131204 this has no responses. How do people use DataLoaders at all on Windows/macOS then?

[adamjstewart]

I think it's just a 10-15 sec overhead per epoch, so it's not that bad, we just get hit particularly hard in our unit tests because we run it many times.

[calebrob6]

Ah okay, that makes a lot more sense, thanks for explaining!

Would parameterizing the tests with num_workers 0 and 1 be sufficient, and, if so, would that speed the tests up to something more acceptable?

[adamjstewart]

1 is almost as bad as 2, anything in parallel incurs the same startup overhead.

[calebrob6]

Only testing 1 instead of both 1 and 2 would cut times by over 50% (from the PyTorch discussion link it looks like "1" is 9 seconds and "2" is 14 seconds) -- do we need to test both?

[calebrob6]

If we don't need to test both, then removing "2" makes sense to me. If there's another reason why we should keep it around, or we do need to test both, then this is also fine.

[adamjstewart]

We don't necessarily need to test 2, I think it's functionally very similar to 1. Either way, we'll still want to skip these tests by default and only run them as integration tests. The tests only take a few seconds on Linux, the only issue is on macOS/Windows. I think we're only going to run the integration tests on Linux anyway.