TripletDataset

[dakshjotwani]

This pull request implements the TripletDataset discussed in #1042. @fmassa @ssnl I look forward to both of you reviewing my work. Please let me know if any additional changes need to be made.

[dakshjotwani]

Also, the travis-ci build is failing because it cannot import IterableDataset. I had to build pytorch from source to be able to import it. How would I go about fixing that?

[dakshjotwani]

Turns out that my testcases won't pass even after removing IterableDataset since the test case checks the __iter__ method, and the __iter__ method uses get_worker_info, which also does not exist in the pip build. All other test cases are passing. Reverting commit.

[fmassa]

Thanks for the PR, I have a few comments.

Also, you can test your changes in CI with latest torchvision by changing the .travis.yaml file.

I don't think you should inherit from FolderDataset, because it's a Dataset, and not an IterableDataset and I'm unsure about the interactions it might have there.

Let me know what you think

[dakshjotwani]

@fmassa @ssnl I have made the requested changes.

In .travis.yml, I changed pytorch to pytorch-nightly to inherit from IterableDataset. By doing this, however, test_cpp_models.py fails (none of my changes are run in this testcase). All tests relevant to my changes pass successfully. How should I go about this?

[fmassa]

@dakshjotwani I'd need to check wrt the test_cpp_models failure, it might be a breaking change in core PyTorch that makes it not compatible.

cc @ShahriarSS if we update the PyTorch version to use the nightly, test_cpp_models seems to fail. Can you have a look?

[ShahriarRezghi]

@fmassa please provide a link where it's failing and I'll look into it.

[dakshjotwani]

@ShahriarSS https://travis-ci.org/pytorch/vision/builds/551800130 is this the link you're looking for?

[ShahriarRezghi]

@dakshjotwani Yes. Thanks

[fmassa]

After some more thoughts, and after seeing @rwightman comment in #1042 (comment), I think we should make this class future-proof for when we want to add hard-negative mining to it.

Let's iterate on the API for it.

Thoughts?

[fmassa]

From the discussion in #1042 (comment), let's hold on merging this until you get some results of performing the online within-batch triplet loss.

[ShahriarRezghi]

@fmassa Apparently the failure of tests are because of a bug here. However I do have a temporary fix for now. Should I wait or apply the fix?

[fmassa]

@ShahriarSS this is then something that should disappear once PyTorch gets fixed, is that right?

[ShahriarRezghi]

@fmassa I would think so. Here is the mention of the problem.

[dakshjotwani]

Closing this for now, since we don't plan on adding a TripletDataset anytime soon.