Some dependencies are incorrect/unnecessary

libffcv / ffcv

FFCV: Fast Forward Computer Vision (and other ML workloads!)

https://ffcv.io

Apache License 2.0

2.82k stars 178 forks source link

Some dependencies are incorrect/unnecessary #159

Closed samuela closed 2 years ago

samuela commented 2 years ago

sklearn is declared as a dependency here. But this should likely be scikit-learn. Also it does not appear to be used anywhere in the actual ffcv source.
matplotlib is declared as a dependency here but does not appear to be used anywhere in the actual ffcv source.
fastargs is declared as a dependency here but is only used in the cifar example code.
imgcat is declared as a dependency here but is not used in the actual ffcv source.
assertpy is declared as a dependency here but is only used in the tests and benchmarks.
psutil is declared as a dependency here but is only used in the tests.
webdataset is declared as a dependency here, but is identified as optional in the code.

samuela commented 2 years ago

numba is not declared as a dependency, but is necessary.

GuillaumeLeclerc commented 2 years ago

Good catch @samuela. Some should definitely be removed. However:

I don't think we want numba to be in the python dependencies as we recommend using it with conda. It includes packages that improve performance over the pip version.
Having the packages required for to run examples was to make it easier for newcomers to just try the library by running the examples and not having to figure out what else they need to install. Do you think it's a bad idea.

Do you want to make a PR ?

samuela commented 2 years ago

I don't have bandwidth to submit a PR atm, but hopefully recording these things here will be useful for someone in the future.

Another one:

OpenCV is not listed as a dependency but it is used here.

samuela commented 2 years ago

Having the packages required for to run examples was to make it easier for newcomers to just try the library by running the examples and not having to figure out what else they need to install. Do you think it's a bad idea.

I would propose having separate dependency lists for the core library, benchmarks, and tests. For example, check out how flax does it here: https://github.com/google/flax/blob/main/setup.py.

GuillaumeLeclerc commented 2 years ago

The thing here is that we want to make it as easy for people to run the experiments. If we add an extra step it will already put a lot of people off using it :/

GuillaumeLeclerc commented 2 years ago

I improved the list in the next release. Reopen if you have another suggestion :)