search

lisa-lab / DeepLearningTutorials

Deep Learning Tutorial notes and code. See the wiki for more info.
http://deeplearning.net/tutorial
Other
4.12k stars 2.13k forks source link

A couple of possible issues with cA code #164

Open gsanroma opened 8 years ago

gsanroma commented 8 years ago

Hi, there might be a couple of issues in the cA code. First, when computing the loss due to the jacobian (line 211) shouldn't the integer division (//) be replaced by a a true division (/) ? Second, the result of the previous line of code is a scalar. Then, why taking its mean when computing the cost function in next line of code (218) ? Thanks, Gerard.

lamblin commented 8 years ago

First, when computing the loss due to the jacobian (line 211) shouldn't the integer division (//) be replaced by a a true division (/) ?

Yes, this should be a true division. This seems to be an issue that has been introduced when trying to make the code compatible with python 3, but that line should not have been changed, since J (and then sum(J ** 2)) should be float.

A pull request to fix that would be welcome.

Second, the result of the previous line of code is a scalar. Then, why taking its mean when computing the cost function in next line of code (218) ?

It is indeed unnecessary, but should not affect the computation speed. That fix would be welcome as well.

Thanks for the report.

gsanroma commented 8 years ago

Is there any tutorial on how to create pull requests ?

lamblin commented 8 years ago

You can have a look at http://deeplearning.net/software/theano/dev_start_guide.html, in particular http://deeplearning.net/software/theano/dev_start_guide.html#installation-and-configuration and http://deeplearning.net/software/theano/dev_start_guide.html#development-workflow, replacing Theano/Theano by lisa-lab/DeepLearningTutorials.