Skip to content
This repository was archived by the owner on Jan 7, 2025. It is now read-only.

Initial support for Torch7 in DIGITS - #324

Merged
lukeyeager merged 3 commits into
NVIDIA:masterfrom
gheinrich:dev/torch2
Oct 2, 2015
Merged

Initial support for Torch7 in DIGITS#324
lukeyeager merged 3 commits into
NVIDIA:masterfrom
gheinrich:dev/torch2

Conversation

@gheinrich

Copy link
Copy Markdown
Contributor

Pull request to prepare ground for merge to master (will need rebasing/squashing and QA report before commit)

@lukeyeager

Copy link
Copy Markdown
Member

Woohoo, great stuff! This is going to take me a while to review. Here are a few things I noticed right away:

Error messages
We need some better error messages for bad installs. I missed the LMDB instructions and got a worthless error in DIGITS when trying to train on an LMDB dataset. And the error in the logs wasn't helpful either. We should (A) find a way to make the docs easier to follow (Julie had this problem too, right?) and (B) have an error like luarock lightningmdb missing instead of something cryptic.

Torch is much slower
This may not be resolvable in the short-term, but I just wanted to make a note that a training job which takes 6sec with Caffe takes 38sec with Torch.

Torch is less accurate
I tried a little test of LeNet on the MNIST test dataset. Look at how different these two graphs are. Does this match what you're seeing on your machine?
Caffe accuracy - 98.1%
torchtesting-caffe-accuracy
Torch accuracy - 94.5%
torchtesting-torch-accuracy

@gheinrich

Copy link
Copy Markdown
Contributor Author

Thanks for the comments. I have pushed a patch which will hopefully make the error message slightly less cryptic. It should show something like:

/fast-scratch/gheinrich/ws/torch_main/install/bin/luajit: ...nrich/ws/torch_main/install/share/lua/5.1/trepl/init.lua:363: /fast-scratch/gheinrich/ws/digits/tools/torch/utils.lua:247: Did you forget to install pb module? c.f. install instructions

Torch is particularly slow on Lenet as Lenet is using the regular cunn backend (as opposed to cudnn). One slight complexity with Torch is that the model definition is not the same depending on which backend you want to use. Originally I wanted a very simple model example where the definition does not depend on whether an Nvidia GPU is available.

Besides, training in Torch is quite slow to start up as the computation of validation loss/accuracy is far from optimal (uses a single thread to load data and run the main computations). On the other hand, training is multi-threaded so we're wasting less time waiting for data.

The difference between Torch and Caffe should be less dramatic on "bigger" models. Still, there are a few things I can do to improve speed. I will try.

Regarding the accuracy: I still can't quite figure out how Caffe manages to be right so quickly. Possibly there are tricks in Caffe to deal with the initial learning rate/momentum in models. When I checked Caffe v.s. Torch on less trivial datasets I had a feeling that the accuracy was somewhat similar.

Thanks again for the comments!

@gheinrich

Copy link
Copy Markdown
Contributor Author

And also, we need to figure out how to run Torch tests on Travis...

@lukeyeager

Copy link
Copy Markdown
Member

And also, we need to figure out how to run Torch tests on Travis...

That should be NBD. Just add a line here: https://github.com/NVIDIA/DIGITS/blob/v2.2.1/.travis.yml#L32

@gheinrich

Copy link
Copy Markdown
Contributor Author

Added Torch install to Travis => code coverage almost unchanged

@gheinrich
gheinrich force-pushed the dev/torch2 branch 3 times, most recently from c850521 to 4e2a55d Compare September 22, 2015 15:35
@lukeyeager

Copy link
Copy Markdown
Member

Yikes, that bumps the TravisCI build time up from ~8mins to ~25mins. But that's actually better than I expected. @semisight and I had a hard time getting the build time under 1 hour, so well done!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One bothersome thing about putting the "experimental" text in the TorchFramework class is that old torch jobs will continue to be flagged as "experimental" even when DIGITS upgrades to "full support" (whatever that means). Is there a way to put the "experimental" flagging in the template somewhere rather than here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'descriptive' name is only used in the navigation pane there on the model creation pane. The model job keeps a link to the framework through the train task and its framework_id field there - the framework ID is what's shown on the home page and in the previous networks tab. We can change the descriptive name and that will change the display on the navigation pane however the framework ID will still be 'torch'. When we want to support Torch9 we can create a new framework ID to denote the change in major version. Is that OK?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see. This isn't a TrainTask, so it doesn't get pickled. Nevermind.

@lukeyeager

Copy link
Copy Markdown
Member

I tried to use AlexNet and I got this error:

torch-alexnet-error

... not super helpful. We need better error reporting. Can you catch Lua errors and format them as an ERROR log message so that DIGITS can catch it and display it?

When I looked in the logs, the issue became clear.

/home/lyeager/torch/install/share/lua/5.1/cudnn/ffi.lua:574: libcudnn.so: cannot open shared object file: No such file or directory
/home/lyeager/torch/install/bin/luajit: /home/lyeager/torch/install/share/lua/5.1/trepl/init.lua:363: /home/lyeager/torch/install/share/lua/5.1/trepl/init.lua:363: /home/lyeager/torch/install/share/lua/5.1/cudnn/ffi.lua:577: 'libcudnn.so not found in library path.
Please install CuDNN from https://developer.nvidia.com/cuDNN
Then make sure all the files named as libcudnn.so* are placed in your library load path (for example /usr/local/lib , or manually add a path to LD_LIBRARY_PATH)
stack traceback:
[C]: in function 'error'
/home/lyeager/torch/install/share/lua/5.1/trepl/init.lua:363: in function 'require'
/home/lyeager/digits/tools/torch/main.lua:175: in main chunk
[C]: in function 'dofile'
...ager/torch/install/lib/luarocks/rocks/trepl/scm-1/bin/th:131: in main chunk
[C]: at 0x00406670

I don't have cuDNN in my path because I use CMake to build Caffe and it sets the rpath for me automatically.

Our current stance is that cuDNN is optional:

For greater performance, you can also install cuDNN.
https://github.com/NVIDIA/DIGITS/blob/v2.2.1/README.md#cuda

But it seems that cuDNN is required to run AlexNet with Torch. Is there any way to get out of requiring cuDNN to run AlexNet?

@lukeyeager

Copy link
Copy Markdown
Member

@lukeyeager - I just wanted to make a note that a training job which takes 6sec with Caffe takes 38sec with Torch.

@gheinrich - The difference between Torch and Caffe should be less dramatic on "bigger" models.

You're right. Here's the results of a [very much non-rigorous] test I tried. It's one epoch on a dataset of ~40k images.

AlexNet (sec) GoogLeNet (sec)
Caffe (v0.13.2, with cuDNN v3 and CNMeM) 126 722
Torch (cuDNN v3) 252 969
Slowdown 2x 1.3x

This isn't a blocking issue for merging this PR, I'm just posting some benchmarking information.

@lukeyeager

Copy link
Copy Markdown
Member

I tried viewing the weights and activations for a Torch GoogLeNet model and got this error:

u"torch classify one task failed with error code 1/home/lyeager/torch/install/bin/luajit: /home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:11: cuda runtime error (2) : out of memory at /home/lyeager/torch/extra/cutorch/lib/THC/THCStorage.cu:44\nstack traceback:\n[C]: in function 'resize'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:11: in function 'torch_Storage_type'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:57: in function 'recursiveType'\n/home/lyeager/torch/install/share/lua/5.1/nn/Module.lua:123: in function 'type'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:45: in function 'recursiveType'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:41: in function 'recursiveType'\n/home/lyeager/torch/install/share/lua/5.1/nn/Module.lua:123: in function 'type'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:45: in function 'recursiveType'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:41: in function 'recursiveType'\n/home/lyeager/torch/install/share/lua/5.1/nn/Module.lua:123: in function 'type'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:45: in function 'recursiveType'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:41: in function 'recursiveType'\n/home/lyeager/torch/install/share/lua/5.1/nn/Module.lua:123: in function 'type'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:45: in function 'recursiveType'\n/home/lyeager/torch/install/share/lua/5.1/nn/utils.lua:41: in function 'recursiveType'\n/home/lyeager/torch/install/share/lua/5.1/nn/Module.lua:123: in function 'cuda'\n/home/lyeager/digits/tools/torch/test.lua:157: in main chunk\n[C]: in function 'dofile'\n...ager/torch/install/lib/luarocks/rocks/trepl/scm-1/bin/th:131: in main chunk\n[C]: at 0x00406670"

@gheinrich

Copy link
Copy Markdown
Contributor Author

We need better error reporting.

I have fixed a bug in the Torch output capture. Now the Lua exceptions should show on the task status box. I have also added a test to verify this automatically (for Caffe and Torch)

@gheinrich

Copy link
Copy Markdown
Contributor Author

Is there any way to get out of requiring cuDNN to run AlexNet?

I will try to change the network definition so that it falls back to the legacy cunn modules if cudnn is not available and nn if no GPU is available

@lukeyeager

Copy link
Copy Markdown
Member

I have fixed a bug in the Torch output capture. Now the Lua exceptions should show on the task status box.

Oh, actually I meant to ask if there was any way to print an ERROR log message so that the process_output() function could store it as the reason for the failure and print something other than "ERROR: error code 1." See here:
https://github.com/NVIDIA/DIGITS/blob/v2.2.1/digits/model/tasks/caffe_train.py#L781-L784

I will try to change the network definition so that it falls back to the legacy cunn modules if cudnn is not available and nn if no GPU is available

Oh yeah, good point. We don't actually require CUDA at the moment either. Would that require writing three different version of each of the standard networks (CPU/CUDA/cuDNN)?

@gheinrich

Copy link
Copy Markdown
Contributor Author

I meant to ask if there was any way to print an ERROR log message so that the process_output() function could store it as the reason for the failure

I think it is possible but it would require some refactoring of the code (to call everything from a pcall - Lua's equivalent of try/except clauses). Maybe we can do it in a further pull request?

Would that require writing three different version of each of the standard networks (CPU/CUDA/cuDNN)?

Hopefully, changes would be fairly limited so it should look like one definition of a model with some aliases to point to the right backend (cudnn, cunn, or nn)

@gheinrich

Copy link
Copy Markdown
Contributor Author

I have updated the definitions of Alexnet/GoogLeNet so they work on CuDNN, bare CUDA and CPU.

@lukeyeager

Copy link
Copy Markdown
Member

I have updated the definitions of Alexnet/GoogLeNet so they work on CuDNN, bare CUDA and CPU.

Verified that it's working for me. Neat!

@lukeyeager

Copy link
Copy Markdown
Member

/cc @soumith @szagoruyko

@szagoruyko

Copy link
Copy Markdown

Hi guys, is there still a problem with torch being slower than caffe? we can try to take a look, I am actually sure that they have the same speed, sometime ago I did testing of both with the same network with https://github.com/szagoruyko/torch-caffe-binding , were giving exactly the same timings with cudnn.

@gheinrich

Copy link
Copy Markdown
Contributor Author

Hi @szagoruyko I have to spend some time studying carefully where the timing discrepancy lives between Torch in DIGITS and Caffe in DIGITS. In the Lua wrapper for Torch in DIGITS I have moved the data loading part to a dedicated thread but I have to check whether we may sometimes be waiting for data in the main thread that is running the forward/backward computations. Besides, for example the flavour of Alexnet we're using in Torch is not strictly identical to the one we're using in Caffe (the Torch one has more filters) so we're not exactly comparing apples to apples. I'll get back to you regarding timings when I have more solid information. Thanks!

@gheinrich

Copy link
Copy Markdown
Contributor Author

Rebase on tip of master and squashed my commits.

@lukeyeager

Copy link
Copy Markdown
Member

It seems to me like everything is working as advertised, but maybe we can advertise the shortcomings a little more clearly?

What if we did something like we did for HDF5 in #226 - provide a little warning that lets people know which things they might expect to be able to do with Torch but can't currently:

  • No multi-GPU support
  • Models don't automatically adjust to images with different sizes/channels
  • ???

@gheinrich

Copy link
Copy Markdown
Contributor Author

maybe we can advertise the shortcomings a little more clearly?

Yes, good point, thanks! I have added a warning to tell the user what the restrictions are when a Torch model is being selected.

The Travis test failed with a cryptic error:

libdc1394 error: Failed to initialize libdc1394

Interestingly, the same commit passed the test on my fork: https://travis-ci.org/gheinrich/DIGITS.

@lukeyeager

Copy link
Copy Markdown
Member

The Travis test failed with a cryptic error

Actually, it looks like another build timeout, even after merging #332. Is Travis getting slower?

https://s3.amazonaws.com/archive.travis-ci.org/jobs/83012672/log.txt

======================================================================
ERROR: test suite for <class 'digits.model.images.classification.test_views.TestTorchLeNet'>
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/miniconda/lib/python2.7/site-packages/nose/suite.py", line 209, in run
    self.setUp()
  File "/home/travis/miniconda/lib/python2.7/site-packages/nose/suite.py", line 292, in setUp
    self.setupContext(ancestor)
  File "/home/travis/miniconda/lib/python2.7/site-packages/nose/suite.py", line 315, in setupContext
    try_run(context, names)
  File "/home/travis/miniconda/lib/python2.7/site-packages/nose/util.py", line 471, in try_run
    return func()
  File "/home/travis/build/NVIDIA/DIGITS/digits/model/images/classification/test_views.py", line 205, in setUpClass
    super(BaseViewsTestWithModel, cls).setUpClass()
  File "/home/travis/build/NVIDIA/DIGITS/digits/model/images/classification/test_views.py", line 129, in setUpClass
    super(BaseViewsTestWithDataset, cls).setUpClass()
  File "/home/travis/build/NVIDIA/DIGITS/digits/model/images/classification/test_views.py", line 83, in setUpClass
    super(BaseViewsTest, cls).setUpClass()
  File "/home/travis/build/NVIDIA/DIGITS/digits/dataset/images/classification/test_views.py", line 154, in setUpClass
    assert cls.dataset_wait_completion(cls.dataset_id) == 'Done', 'create failed'
AssertionError: create failed

I restarted your build for now.

@lukeyeager

Copy link
Copy Markdown
Member

I have added a warning to tell the user what the restrictions are when a Torch model is being selected.

Looks good, thanks.

lukeyeager added a commit that referenced this pull request Oct 2, 2015
Initial support for Torch7 in DIGITS
@lukeyeager
lukeyeager merged commit 9f063db into NVIDIA:master Oct 2, 2015
@lukeyeager

Copy link
Copy Markdown
Member

Merged. Many thanks to @Sravan2j for the initial work and to @gheinrich for bringing it home!

TODO:

@soumith

soumith commented Oct 2, 2015

Copy link
Copy Markdown

whut whut!!!!!!!!!!!!!!!!!!!!!!

@j-wilson

j-wilson commented Oct 2, 2015

Copy link
Copy Markdown

Please do let us / rest of the dev community know what issues are
outstanding / what features have yet to be implemented. I've been praying
for Torch support to go through for half a year now; so, I'm (obviously)
very excited and would be happy to help out.

  • james

On Fri, Oct 2, 2015 at 10:21 PM, Soumith Chintala notifications@github.com
wrote:

whut whut!!!!!!!!!!!!!!!!!!!!!!


Reply to this email directly or view it on GitHub
#324 (comment).

@lukeyeager

Copy link
Copy Markdown
Member

Please do let us / rest of the dev community know what issues are
outstanding / what features have yet to be implemented.

@j-wilson, look for outstanding issues at torch .

Please try it out and let us know how it goes!

@gheinrich

Copy link
Copy Markdown
Contributor Author

You might want to check Torch install/usage instructions for DIGITS there. Any feedback welcome, thanks!

@j-wilson

j-wilson commented Oct 3, 2015

Copy link
Copy Markdown

Probably a simple fix for this,
luarocks install "https://raw.github.com/deepmind/torch-hdf5/master/hdf5-0-0.rockspec"
fails with:

CMake Error at /usr/local/Cellar/cmake/3.3.1/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:148 (message):
  Could NOT find HDF5: Found unsuitable version "", but required is at least
  "1.8" (found HDF5_hdf5_LIBRARY-NOTFOUND)
Call Stack (most recent call first):
  /usr/local/Cellar/cmake/3.3.1/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:386 (_FPHSA_FAILURE_MESSAGE)
  /usr/local/Cellar/cmake/3.3.1/share/cmake/Modules/FindHDF5.cmake:375 (find_package_handle_standard_args)
  CMakeLists.txt:4 (FIND_PACKAGE)

The version of HDF5 I have installed (h5py-2.2.1) was part of the requirements for DIGITS.

@gheinrich

Copy link
Copy Markdown
Contributor Author

Did you also install package libhdf5-serial-dev as part part of the Caffe install instructions there?
In case it isn't clear from the DIGITS install instructions there, Caffe still needs to be installed even if you're only planning to use Torch as DIGITS still depends on Caffe for various things (we'd like to lift this limitation in the future but it will require some rework).

@gheinrich
gheinrich deleted the dev/torch2 branch October 4, 2015 09:59
@gheinrich

Copy link
Copy Markdown
Contributor Author

@j-wilson did that work for you?

@j-wilson

j-wilson commented Oct 8, 2015

Copy link
Copy Markdown

@gheinrich After installing Caffe, the luarocks install went through without issue. Starting up a devserver asks for a path to Caffe (no mention of Torch). Been a bit busy; so, I haven't had a chance to chase this further. At this point, I'd imagine that it's probably a simple issue of providing the proper paths etc. Will follow up.

@gheinrich

Copy link
Copy Markdown
Contributor Author

Thanks @j-wilson, if the th executable is in your path DIGITS will find it and enable Torch support and "Torch" tabs should appear in the network selection panel on the model creation page. If th is not in your path then DIGITS will silently disable support for Torch. I implemented it this way because Torch is optional in DIGITS. However if you want to point Torch to a specific path for the th executable you may do so by typing digits-devserver -c and DIGITS will ask:

==================================== Torch =====================================
Where is torch installed?

    Suggested values:
    (*)  [Previous]       <PATHS>
    (P)  [PATH/TORCHPATH] <PATHS>
    (N)  [none]           <NONE>
>> 

I hope this is clear enough - there is an attempt at explaining this there but it's probably hidden too deep :-)

@j-wilson

j-wilson commented Oct 8, 2015

Copy link
Copy Markdown

Interesting. So th is definitely in my path (in the present case it's located in /usr/local/bin). Does Torch support require a built version of Caffe or simply the associated Caffe dependencies?

@gheinrich

Copy link
Copy Markdown
Contributor Author

If you were able to start the DIGITS server then all the Caffe dependencies should be OK and you should be able to use Torch to train models in DIGITS as illustrated on https://github.com/NVIDIA/DIGITS/blob/master/docs/images/torch-selection.png. You need to click on the Torch tab underneath 'Standard Networks'. Good luck!

@lukeyeager

Copy link
Copy Markdown
Member

In case this needs clearing up:

  • Torch doesn't require Caffe. But we make the assumption in the Torch install instruction that all of the Caffe install instructions have been followed already.
  • If digits-devserver --config doesn't ask you for the location of torch, then you need to upgrade DIGITS to include the changes in this pull request.

@soumith

soumith commented Oct 8, 2015

Copy link
Copy Markdown

I successfully installed digits with torch support. I realized that i deleted my imagenet lmdb db, so figuring out how to recreate it, after which I'll try to close the perf gaps.

@gheinrich

Copy link
Copy Markdown
Contributor Author

Thanks Soumith. Using the CUDA profiler we have established that the perf gap between Caffe and Torch in DIGITS is mainly due to the data loader we have implemented around torch.cudnn. The amount of CUDA compute is almost identical between Caffe and Torch for a given network. As Natalia Gimelshein explained to me, there is one tiny difference (which amounts to ~5% of extra compute utlization in Torch) through extra calls to convolve_dgrad_engine which according to Natalia could be avoided by setting the gradInput field of the first layer in the network to nil. It doesn't quite work for nn.MulConstant though (maybe I should set inplace=true?).
The Torch data loader in DIGITS needs some rework. Currently it is operating from a single separate thread and when using LMDB it is not able to load data sufficiently quickly to completely operate in the shadow of the main computations. When using HDF5 the data are loaded upfront so it takes a while before we can start training. It would be nice to implement a multi-threaded data loader that can load data on the go for both LMDB and HDF5.
Besides, in the current tip of the master branch there are some unnecessary computations going on in the critical path on CPU (I have a pending commit to make them optional and disabled by default - this saves ~20% on training time on LeNet+MNIST).

@soumith

soumith commented Oct 21, 2015

Copy link
Copy Markdown

I've finally got an lmdb (took long enough) and can take a look slowly.

An example multi-threaded lmdb data-loader is here btw: https://github.com/facebook/eyescream/blob/master/lsun/donkey_lsun.lua

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants