Skip to content

C++ Caltech Dataset #1121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

ShahriarRezghi
Copy link
Contributor

@ShahriarRezghi ShahriarRezghi commented Jul 15, 2019

Description:

  • Datasets mainly need filesystem to operate. C++17 has filesystem but since we are using C++11 I had to write it with system calls and since I don't have a working windows environment I have only written POSIX side (doesn't work on windows but works fine on others).
  • For reading images I have used OpenCV. Python models take in a transform function that does some operations on a PILL image. I have written a similar function that takes a cv::Mat and returns the matrix after doing operations on it. there are some default transform functions in the namespace cv_transforms that convert image to rgb or grayscale and resize to 224x224.
  • Right now I have only written Caltech101 dataset. If we agree on the structure I can write the rest.
  • Right now there is no method of downloading the datasets directly in C++. We can use a library like boost or others. Or we can write one from scratch with socket programming. Or we can call python functions (which I don't think is a good idea) (haven't decided yet).
  • The code is close to python in the internals and API of datasets. It does vary in utilities.
  • Also I have moved global.h up to include it in datasets too.
  • For tests we can create fake data in Python and call C++ functions on it using bindings to test. Or we can do the whole thing in C++ and use gtest (haven't decided yet).

@codecov-io
Copy link

codecov-io commented Jul 15, 2019

Codecov Report

Merging #1121 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1121   +/-   ##
=======================================
  Coverage   65.78%   65.78%           
=======================================
  Files          79       79           
  Lines        5834     5834           
  Branches      887      887           
=======================================
  Hits         3838     3838           
  Misses       1726     1726           
  Partials      270      270

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2287c8f...adc716d. Read the comment docs.

@fmassa
Copy link
Member

fmassa commented Jul 26, 2019

Hi @ShahriarSS

Thanks for the PR!

Here are some initial thoughts:

Use of OpenCV:

The C++ models that we have are compatible with the Python models. OpenCV return the images in a different format than what we generally use (BGR in 0-255, instead of RGB in 0-1).
I would like the C++ frontend to be equivalent to the Python frontend.
This might mean writing a few compatibility functions (maybe using OpenCV, maybe not) that returns the images, and perform image operations on them, in the same format as in the Python frontend.

I'm currently reworking the transforms to also accept PyTorch tensors. This means that we could also do the same thing in C++, and use torch ops for rescaling / etc. The question now is to have a basic read-image function that is compatible with the rest of the codebase.

Also, can we make the API of the methods require OpenCV? For example, we should avoid passing and returning cv::Mat objects, and instead use Tensors. This would make changing the backend much simpler.

What about the following: Could you split this PR into two components:

  • the data reading abstractions, with minimal functions for reading an image, and not exposing OpenCV in the API
  • the dataset itself, which leverages the data reading functionality.

Filesystem access

We have a large user-base of Windows users, and it might be worth considering if we want to add support for Windows right in the beginning or not. I'm ccing @soumith on this

Download inside datasets

I don't think this is a requirement for now.

Tests

Ideally we would use the same Python functions that create fake datasets, save them to disk and read with the C++ frontend to verify that everything works as expected.
I think it might be better to just write the tests straight in C++, to avoid having to expose a number of things in Python (and having to write the binding code just to test it)

@yassineAlouini
Copy link
Contributor

Thanks @ShahriarSS for this contribution and sorry for the late reply.

The datasets API is being rewritten into a new API as described here.

Not yet sure if it includes C++ datasets (or if it is planned) as well but I suppose it does for now (or will shortly). @pmeier is there a README that details the migration process for C++ datasets? Or maybe this only concerns Python ones?

I would suggest the following:

What do you think @ShahriarSS about that? Feel free to ask additional questions, thanks!

@pmeier
Copy link
Collaborator

pmeier commented May 3, 2022

Not yet sure if it includes C++ datasets (or if it is planned) as well but I suppose it does for now (or will shortly)

There are no plans to have C++ wrappers for the datasets. Looking at this PR, the data reading, i.e. image decoding part in C++, is already implemented in torchvision.io.

Given that this PR is stale for ~3 years, I'll close this for now. @ShahriarSS if there is still need for the C++ dataset wrappers, would you mind opening an issue?

@pmeier pmeier closed this May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants