Add Dali MNIST example #3721

irustandi · 2020-09-29T10:58:57Z

What does this PR do?

Add a modification of the MNIST example using NVIDIA DALI.

Fixes #791

Before submitting

Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
Did you read the contributor guideline, Pull Request section?
Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
Did you make sure to update the documentation with your changes?
Did you write any new necessary tests?
Did you verify new and existing tests pass locally with your changes?
If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

pep8speaks · 2020-09-29T10:59:00Z

Hello @irustandi! Thanks for updating this PR.

In the file pl_examples/basic_examples/mnist_dali.py:

Line 87:121: E501 line too long (125 > 120 characters)

Comment last updated at 2020-11-06 13:45:49 UTC

codecov · 2020-09-29T11:09:58Z

Codecov Report

Merging #3721 into master will increase coverage by 1%.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #3721   +/-   ##
======================================
+ Coverage      93%     93%   +1%     
======================================
  Files         116     116           
  Lines        8883    8815   -68     
======================================
- Hits         8233    8225    -8     
+ Misses        650     590   -60

carmocca · 2020-09-29T13:51:13Z

Can you run black on it?

pre-commit run --files pl_examples/basic_examples/mnist_dali.py

Borda

can you add test as calling this CLI in examples/test_...py

irustandi · 2020-09-29T15:20:45Z

Changes made as requested. Need to make sure that DALI (with the appropriate CUDA version) is available in the test environment. How can we do this?

Borda

it would be nice to add some short docstrings to the class instead of general comments

Borda

pls add the package to the requirements
probably it has to be compiled from the source... https://docs.nvidia.com/deeplearning/dali/user-guide/docs/compilation.html

irustandi · 2020-09-30T11:13:53Z

pls add package to requirements

A couple of questions regarding this. According to DALI doc, I would need to install it like this:

pip install --extra-index-url https://developer.download.nvidia.com/compute/redist nvidia-dali-cuda100

How can I add it to requirements in this case? And also, is CUDA 10 OK?

Borda · 2020-09-30T11:21:55Z

Collecting nvidia-dali-cuda100
  Downloading nvidia-dali-cuda100-0.0.1.dev4.tar.gz (3.9 kB)
    ERROR: Command errored out with exit status 1:
     command: /Users/jirka/Applications/venv_PL/bin/python -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/private/var/folders/c1/fc_9d4b10c1cwmsm0l6cgzxc0000gn/T/pip-install-h606em3s/nvidia-dali-cuda100/setup.py'"'"'; __file__='"'"'/private/var/folders/c1/fc_9d4b10c1cwmsm0l6cgzxc0000gn/T/pip-install-h606em3s/nvidia-dali-cuda100/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /private/var/folders/c1/fc_9d4b10c1cwmsm0l6cgzxc0000gn/T/pip-pip-egg-info-g7b2_uvu
         cwd: /private/var/folders/c1/fc_9d4b10c1cwmsm0l6cgzxc0000gn/T/pip-install-h606em3s/nvidia-dali-cuda100/
    Complete output (18 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/private/var/folders/c1/fc_9d4b10c1cwmsm0l6cgzxc0000gn/T/pip-install-h606em3s/nvidia-dali-cuda100/setup.py", line 150, in <module>
        raise RuntimeError(open("ERROR.txt", "r").read())
    RuntimeError:
    ###########################################################################################
    The package you are trying to install is only a placeholder project on PyPI.org repository.
    This package is hosted on NVIDIA Python Package Index.
    
    This package can be installed as:
    ```
    $ pip install nvidia-pyindex
    $ pip install nvidia-dali-cuda100
    ```
    
    Please refer to NVIDIA DALI installation guide for instructions:
    https://docs.nvidia.com/deeplearning/dali/user-guide/docs/installation.html
    ###########################################################################################
    ----------------------------------------
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.

may be linked that I do not have gpu...

irustandi · 2020-09-30T11:51:52Z

With --extra-index-url added to requirements/examples.txt, the CI complete testing can get past the dependency installation step, however, it fails in the actual test since CUDA does not seem to be available (libcuda.so is missing). Also, DALI does not seem to be supported in Windows and Mac. Install pkg fails because the --extra-index-url line is somehow is not the expected format.

irustandi · 2020-09-30T12:38:22Z

To go forward on this, I see the following needs at the very least:

Ability to make the DALI example be tested on Linux only (exclude Windows and Mac)
Support CUDA in CI

Let me know how I should proceed.

Borda · 2020-09-30T13:15:23Z

@irustandi sure, that is what I wanted to suggest, mark the test as GPU needed and then we need to include Dali in our CUDA Docker image

…s.py

mergify · 2020-09-30T13:39:15Z

This pull request is now in conflict... :(

Borda · 2020-11-04T20:53:32Z

requirements/examples.txt

+gym>=0.17.0
+nvidia-dali --extra-index-url https://developer.download.nvidia.com/compute/redist


we shall install it at least on our Docker image from the source

Traceback (most recent call last): File "<string>", line 1, in <module> File "/private/var/folders/c1/fc_9d4b10c1cwmsm0l6cgzxc0000gn/T/pip-install-xo27f9qp/nvidia-dali-cuda100/setup.py", line 150, in <module> raise RuntimeError(open("ERROR.txt", "r").read()) RuntimeError: ########################################################################################### The package you are trying to install is only a placeholder project on PyPI.org repository. This package is hosted on NVIDIA Python Package Index. This package can be installed as: ``` $ pip install nvidia-pyindex $ pip install nvidia-dali-cuda100 ``` Please refer to NVIDIA DALI installation guide for instructions: https://docs.nvidia.com/deeplearning/dali/user-guide/docs/installation.html ###########################################################################################

cc: @ydcjeff

Yeah we won't be able to include this as a requirement here, but I think I'll add a note in the docs

… install DALI

Borda · 2020-11-05T16:00:21Z

.drone.yml

@@ -32,6 +32,7 @@ steps:
    - pip --version
    - nvidia-smi
    - pip install -r ./requirements/devel.txt --upgrade-strategy only-if-needed -v --no-cache-dir
+    - pip install --extra-index-url https://developer.download.nvidia.com/compute/redist nvidia-dali-cuda100


testing now if the example runs, later we shall add it to the used docker image

pl_examples/basic_examples/mnist_dali.py

…ightning into dali_support

SeanNaren · 2020-11-05T19:09:48Z

Let's wait till all tests are passing, some weirdness with one job...

awaelchli · 2020-11-05T22:52:05Z

correct me if I'm wrong, this PR does not add any additional support for Dali, it only adds examples. Consider renaming the PR title.

tchaton

Great PR ! minors changes.

pl_examples/basic_examples/mnist_dali.py

requirements/examples.txt

SeanNaren · 2020-11-06T14:54:08Z

Thanks @irustandi for the work, apologies on the delay. NVIDIA DALI is awesome, hoping to see more integration into loaders :)

* add MNIST DALI example, update README.md * Fix PEP8 warnings * reformatted using black * add mnist_dali to test_examples.py * Add documentation as docstrings * add nvidia-pyindex and nvidia-dali-cuda100 * replace nvidia-pyindex with --extra-index-url * mark mnist_dali test as Linux and GPU only * adjust CUDA docker and examples.txt, fix import error in test_examples.py * adjust the GPU check * Exit when DALI is not available * remove requirements-examples.txt and DALI pip install * Refactored example, moved to new logging api, added runtime check for test and dali script * Patch to reflect the mnist example module * add req. * Apply suggestions from code review * Removed requirement as it breaks CPU install, added note in README to install DALI * add DALI to Drone * test examples * Apply suggestions from code review * imports * ABC * cuda * cuda * pip DALI * Move build into init function Co-authored-by: SeanNaren <sean@grid.ai> Co-authored-by: Jirka Borovec <jirka@pytorchlightning.ai> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Sean Naren <sean.narenthiran@gmail.com>

irustandi added 2 commits September 29, 2020 06:46

add MNIST DALI example, update README.md

29bdc12

Merge remote-tracking branch 'upstream/master' into dali_support

dbef9de

mergify bot requested a review from a team September 29, 2020 10:59

Borda added feature Is an improvement or enhancement example labels Sep 29, 2020

Fix PEP8 warnings

743afb7

Borda requested changes Sep 29, 2020

View reviewed changes

mergify bot requested a review from a team September 29, 2020 14:10

irustandi added 2 commits September 29, 2020 11:06

reformatted using black

cd9c892

add mnist_dali to test_examples.py

221fe9b

Borda self-requested a review September 29, 2020 15:53

Borda approved these changes Sep 29, 2020

View reviewed changes

mergify bot requested a review from a team September 29, 2020 15:55

Add documentation as docstrings

4b4ebe9

Borda requested changes Sep 30, 2020

View reviewed changes

mergify bot requested a review from a team September 30, 2020 11:02

irustandi added 2 commits September 30, 2020 07:35

add nvidia-pyindex and nvidia-dali-cuda100

4cb797e

replace nvidia-pyindex with --extra-index-url

3b3a5dd

irustandi added 2 commits September 30, 2020 09:25

mark mnist_dali test as Linux and GPU only

31fa2a9

adjust CUDA docker and examples.txt, fix import error in test_example…

daa9a4b

…s.py

Borda reviewed Nov 4, 2020

View reviewed changes

Removed requirement as it breaks CPU install, added note in README to…

832b5e0

… install DALI

Borda self-requested a review November 4, 2020 22:46

Merge branch 'master' into dali_support

6effea0

ydcjeff mentioned this pull request Nov 5, 2020

[dockers] install nvidia-dali-cudaXXX #4532

Merged

4 tasks

add DALI to Drone

7751cbd

Borda reviewed Nov 5, 2020

View reviewed changes

test examples

6472e7f

Borda reviewed Nov 5, 2020

View reviewed changes

pl_examples/basic_examples/mnist_dali.py Outdated Show resolved Hide resolved

pl_examples/basic_examples/mnist_dali.py Show resolved Hide resolved

Borda and others added 7 commits November 5, 2020 18:36

Apply suggestions from code review

abb1d6b

imports

a6223aa

ABC

3d3e75f

Merge branch 'master' into dali_support

7b468e2

cuda

61da5e1

Merge branch 'dali_support' of https://github.com/irustandi/pytorch-l…

67b8e2f

…ightning into dali_support

cuda

c5cb549

Borda approved these changes Nov 5, 2020

View reviewed changes

SeanNaren approved these changes Nov 5, 2020

View reviewed changes

SeanNaren changed the title ~~Dali support~~ Add Dali MNIST example Nov 5, 2020

SeanNaren and others added 2 commits November 5, 2020 22:56

Merge branch 'master' into dali_support

de6f433

pip DALI

8c09298

tchaton requested changes Nov 6, 2020

View reviewed changes

pl_examples/basic_examples/mnist_dali.py Outdated Show resolved Hide resolved

requirements/examples.txt Show resolved Hide resolved

SeanNaren and others added 2 commits November 6, 2020 11:33

Merge branch 'master' into dali_support

f3408b3

Move build into init function

f7afb45

tchaton approved these changes Nov 6, 2020

View reviewed changes

SeanNaren merged commit 6e5f232 into Lightning-AI:master Nov 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Dali MNIST example #3721

Add Dali MNIST example #3721

irustandi commented Sep 29, 2020

pep8speaks commented Sep 29, 2020 •

edited

Loading

codecov bot commented Sep 29, 2020 •

edited

Loading

carmocca commented Sep 29, 2020

Borda left a comment

irustandi commented Sep 29, 2020

Borda left a comment

Borda left a comment •

edited

Loading

irustandi commented Sep 30, 2020

Borda commented Sep 30, 2020

irustandi commented Sep 30, 2020

irustandi commented Sep 30, 2020

Borda commented Sep 30, 2020

mergify bot commented Sep 30, 2020

Borda Nov 4, 2020

SeanNaren Nov 4, 2020

Borda Nov 5, 2020

SeanNaren commented Nov 5, 2020

awaelchli commented Nov 5, 2020

tchaton left a comment

SeanNaren commented Nov 6, 2020

		gym>=0.17.0
		nvidia-dali --extra-index-url https://developer.download.nvidia.com/compute/redist

Add Dali MNIST example #3721

Add Dali MNIST example #3721

Conversation

irustandi commented Sep 29, 2020

What does this PR do?

Before submitting

PR review

Did you have fun?

pep8speaks commented Sep 29, 2020 • edited Loading

Comment last updated at 2020-11-06 13:45:49 UTC

codecov bot commented Sep 29, 2020 • edited Loading

Codecov Report

carmocca commented Sep 29, 2020

Borda left a comment

Choose a reason for hiding this comment

irustandi commented Sep 29, 2020

Borda left a comment

Choose a reason for hiding this comment

Borda left a comment • edited Loading

Choose a reason for hiding this comment

irustandi commented Sep 30, 2020

Borda commented Sep 30, 2020

irustandi commented Sep 30, 2020

irustandi commented Sep 30, 2020

Borda commented Sep 30, 2020

mergify bot commented Sep 30, 2020

Borda Nov 4, 2020

Choose a reason for hiding this comment

SeanNaren Nov 4, 2020

Choose a reason for hiding this comment

Borda Nov 5, 2020

Choose a reason for hiding this comment

SeanNaren commented Nov 5, 2020

awaelchli commented Nov 5, 2020

tchaton left a comment

Choose a reason for hiding this comment

SeanNaren commented Nov 6, 2020

pep8speaks commented Sep 29, 2020 •

edited

Loading

codecov bot commented Sep 29, 2020 •

edited

Loading

Borda left a comment •

edited

Loading