made tutorials compatible with python 3 #132

gyom · 2016-01-22T20:03:33Z

All the examples from the main page (http://deeplearning.net/tutorial/) have been updated, except for two.

They have all been tested, to then extent that they run fine with python 2 and python 3, for 10-20 minutes, but I didn't have a GPU available to run them to completion (potential source for errors in final lines of scripts).

rnnrbm.py has an issue with the "midi" module that is downloaded from an external source. I am led to believe that it is not compatible with python 3 due to the way that it loads local sources. Not sure if this is fixable without modifying the "midi" module.

With rnnslu.py, the existing code currently fails to run on python 2 without any changes to the code. Not sure if Gregoire Mesnil is responsible for maintaining this code. For one thing, the script is referring to the url http://www-etud.iro.umontreal.ca/~mesnilgr/atis/conlleval.pl which is currently broken.

lamblin · 2016-01-26T01:02:12Z

Fix #114.

lamblin · 2016-01-26T23:05:31Z

Two tests are failing because of a change in Theano, in some place we did not expect code to use directly: the Param class has been deleted, since the In class now supports all its features. See Theano/Theano#3890
While I think we should be nice to keep some compatibility if possible (we'll discuss that), it would still be a good idea to update the tutorials.

For conlleval, you can download a copy at http://www-etud.iro.umontreal.ca/~lamblinp/code/conlleval.pl so you can run (and port) rnnslu.py (not to replace the URL). We will try to find a better hosting for that script.

Could you do that this week?

gyom · 2016-01-27T16:36:29Z

I'll do this Thursday to be able to talk to you (Pascal and Fred) in person about issues that come up. Are you going to be at the lab ?

lamblin · 2016-01-27T16:47:05Z

I'm going to be there, but Fred is away until next week.

gyom · 2016-01-28T21:28:32Z

Made changes from Fred's pull request.

Also fixed rnnslu.py. Right now, this requires manually downloading conlleval.py into code/rnnslu/conlleval.py before running the script.

lamblin · 2016-01-28T22:39:21Z

code/SdA.py

                c.append(pretraining_fns[i](index=batch_index,
                         corruption=corruption_levels[i],
                         lr=pretrain_lr))
-            print 'Pre-training layer %i, epoch %d, cost ' % (i, epoch),
-            print numpy.mean(c)
+            print('Pre-training layer %i, epoch %d, cost ' % (i, epoch))


This is not equivalent to the previous call: the previous call was printing something without a newline at the end, so that the next print statement would append to the same line.
The equivalent for the new print() function would be:
print('Pre-training layer %i, epoch %d, cost ' % (i, epoch), end='')

lamblin · 2016-01-28T22:47:43Z

Other than that, it looks good to me, but I'll let @nouiz do a final review.

nouiz · 2016-01-29T18:29:51Z

I'll do a final review when @lamblin comment are addressed. thanks

On Thu, Jan 28, 2016 at 5:47 PM, Pascal Lamblin notifications@github.com
wrote:

Other than that, it looks good to me, but I'll let @nouiz
https://github.com/nouiz do a final review.

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

nouiz · 2016-02-01T14:20:08Z

I don't get a notification when a new commit is added to a PR. Add a comment to the PR when you want one of us to review it again.

nouiz · 2016-02-01T14:24:49Z

code/utils.py

@@ -7,6 +7,7 @@
 """


+from six.moves import xrange


I think it will be a better example to follow if we don't use this import and change everywhere xrange to range. Can you do that?

I'll change all the instances with range instead of xrange. That's what I did at first, but then I figured the proper way to use xrange in a compatible way. I'll just use range for all cases. They're "safe" in the sense that none of those calls require fast execution.

nouiz · 2016-02-01T14:28:10Z

Can you add this in the .travis file to have travis run with python 3?

language: python
python:
  - "2.6"
  - "3.3"

What is the problem with rnnrbm? The midi related files? For this one can you download this zip file: http://www.iro.umontreal.ca/~lisa/deep/midi.zip and make a new zip with the files updated to python 3? I will then update it on the server. (and remove the command related to it)

thanks

gyom · 2016-02-01T18:35:27Z

The issue with the rnnrbm is that the midi library is not loading properly in python 3 because it uses one way to refer to its internal files instead of the python 3 way. I spoke with Pascal about this, and he knows better what I'm talking about. If we can update the midi.zip file, then it's possible to fix this, and this is what I'll do.

nouiz · 2016-02-01T19:31:46Z

thanks

On Mon, Feb 1, 2016 at 1:35 PM, Guillaume Alain notifications@github.com
wrote:

The issue with the rnnrbm is that the midi library is not loading properly
in python 3 because it uses one way to refer to its internal files instead
of the python 3 way. I spoke with Pascal about this, and he knows better
what I'm talking about. If we can update the midi.zip file, then it's
possible to fix this, and this is what I'll do.

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

lamblin · 2016-02-03T00:11:46Z

code/hmc/test_hmc.py

+except:
+    # python 3 compatibility
+    # http://stackoverflow.com/questions/3073259/python-nose-import-error
+    from hmc.hmc import HMC_sampler


So I have more insight about this.
In this case, the issue is a clash of names between the hmc/ directory and the hmc.py file inside. On Python 3, nosetests thinks hmc is the directory, but the regular Python interpreter thinks it is hmc.py (if you try to import test_hmc for instance, or run python test_hmc.py).
This looks like an issue with nosetests to me. Possible solutions would include:

Yours (except that I would catch only ImportError rather than anything)

Having different names, by either renaming hmc/ or hmc.py. That would break direct links, though.

I would prefer the first one.
However, for midi, I'm not sure that the issue is the same exactly.

I also prefer option 1.

lamblin · 2016-02-03T20:29:47Z

For conlleval, can you try downloading it from http://www.cnts.ua.ac.be/conll2000/chunking/conlleval.txt ?

abergeron · 2016-02-03T21:13:26Z

conlleval has already been fixed in master.

nouiz · 2016-02-03T21:21:37Z

Also, this PR need a rebase, it have conflict with master.

gyom · 2016-02-03T21:29:41Z

In terms of language compatibility and imported modules, I have made the "midi" module work with python 2 and 3. Now, the problem is that it has difficulties loading midi files. It works fine in python 2, but in python 3 I get this error.

Traceback (most recent call last):
File "code/rnnrbm.py", line 298, in
model = test_rnnrbm()
File "code/rnnrbm.py", line 294, in test_rnnrbm
batch_size=batch_size, num_epochs=num_epochs)
File "code/rnnrbm.py", line 248, in train
for f in files]
File "code/rnnrbm.py", line 248, in
for f in files]
File "/u/alaingui/Documents/DeepLearningTutorials/code/midi/utils.py", line 23, in init
midi_in.read()
File "/u/alaingui/Documents/DeepLearningTutorials/code/midi/MidiInFile.py", line 47, in read
p.parseMThdChunk()
File "/u/alaingui/Documents/DeepLearningTutorials/code/midi/MidiFileParser.py", line 48, in parseMThdChunk
raise TypeError("It is not a valid midi file!")
TypeError: It is not a valid midi file!

At first glance, this seems like it's a problem when decoding the raw files. This might be due to byte array encodings, with strings being used for one purpose in python 2 that just doesn't translated well into python 3.

I'm signaling this because I'm now no longer strictly in the "straightforward port from python 2 to 3", but probably more into the development of the "midi" module where I'm required to figure out what the code does and what it's trying to do to decode midi files. The question becomes whether this falls outside of the scope of the original ticket or not.

I won't commit this "midi" code in the DeepLearningTutorials in any case, but I can put it elsewhere if you want to have a look at it.

nouiz · 2016-02-03T23:44:42Z

Can you give us access to that midi version?

On Wed, Feb 3, 2016 at 4:29 PM, Guillaume Alain notifications@github.com
wrote:

In terms of language compatibility and imported modules, I have made the
"midi" module work with python 2 and 3. Now, the problem is that it has
difficulties loading midi files. It works fine in python 2, but in python 3
I get this error.

Traceback (most recent call last):
File "code/rnnrbm.py", line 298, in
model = test_rnnrbm()
File "code/rnnrbm.py", line 294, in test_rnnrbm
batch_size=batch_size, num_epochs=num_epochs)
File "code/rnnrbm.py", line 248, in train
for f in files]
File "code/rnnrbm.py", line 248, in
for f in files]
File "/u/alaingui/Documents/DeepLearningTutorials/code/midi/utils.py",
line 23, in init
midi_in.read()
File
"/u/alaingui/Documents/DeepLearningTutorials/code/midi/MidiInFile.py", line
47, in read
p.parseMThdChunk()
File
"/u/alaingui/Documents/DeepLearningTutorials/code/midi/MidiFileParser.py",
line 48, in parseMThdChunk
raise TypeError("It is not a valid midi file!")
TypeError: It is not a valid midi file!

At first glance, this seems like it's a problem when decoding the raw
files. This might be due to byte array encodings, with strings being used
for one purpose in python 2 that just doesn't translated well into python 3.

I'm signaling this because I'm now no longer strictly in the
"straightforward port from python 2 to 3", but probably more into the
development of the "midi" module where I'm required to figure out what the
code does and what it's trying to do to decode midi files. The question
becomes whether this falls outside of the scope of the original ticket or
not.

I won't commit this "midi" code in the DeepLearningTutorials in any case,
but I can put it elsewhere if you want to have a look at it.

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

…to completion

…module to make it compatible with python 3

…but not to completion

nouiz · 2016-02-08T19:40:45Z

travis tests are failing due to that:

File "/home/travis/build/lisa-lab/DeepLearningTutorials/code/rnnslu.py", line 144
print stdout.split('\n')

this is the new change in master that requested a rebase.

gyom · 2016-02-09T16:35:51Z

Should be fixed. We'll see what travis reports.

nouiz · 2016-02-09T21:18:19Z

thanks. merging.

made tutorials compatible with python 3

made tutorials compatible with python 3 Former-commit-id: 2efc6cb

lamblin reviewed Jan 28, 2016
View reviewed changes

nouiz reviewed Feb 1, 2016
View reviewed changes

lamblin mentioned this pull request Feb 2, 2016

Use the In object as Param is deprecated #133

Merged

lamblin reviewed Feb 3, 2016
View reviewed changes

gyom added 6 commits February 8, 2016 14:30

successfully ported logistic_sgd.py

d2764f2

fixed everything except rnnrbm and rnnslu, partial tests run but not …

2c022d1

…to completion

partly fixed rnnrbm, but we will need to do some magic with the midi …

53f246d

…module to make it compatible with python 3

made rnnslu compatible with python 3. tested on cpu for many epochs, …

2c610d3

…but not to completion

all fixes suggested by Pascal, plus update for the doc

226729f

travis python version update as suggested by Fred

90b925b

gyom added 2 commits February 8, 2016 14:32

got rid of all the xrange

4c0858d

minor edit to respond to Pascal's suggestion

dcfe518

missed one print statement

8ca9239

nouiz added a commit that referenced this pull request Feb 9, 2016

Merge pull request #132 from gyom/master

1bb3230

made tutorials compatible with python 3

nouiz merged commit 1bb3230 into lisa-lab:master Feb 9, 2016

nouiz mentioned this pull request Feb 9, 2016

Not compatible with Python 3+ , Pl. start a development branch #114

Closed

taneishi pushed a commit to taneishi/DBN that referenced this pull request Nov 28, 2019

Merge pull request lisa-lab#132 from gyom/master

2efc6cb

made tutorials compatible with python 3

taneishi pushed a commit to taneishi/DBN that referenced this pull request Feb 13, 2020

Merge pull request lisa-lab#132 from gyom/master

d5ec569

made tutorials compatible with python 3 Former-commit-id: 2efc6cb

made tutorials compatible with python 3 #132

made tutorials compatible with python 3 #132

Conversation

gyom commented Jan 22, 2016

Uh oh!

lamblin commented Jan 26, 2016

Uh oh!

lamblin commented Jan 26, 2016

Uh oh!

gyom commented Jan 27, 2016

Uh oh!

lamblin commented Jan 27, 2016

Uh oh!

gyom commented Jan 28, 2016

Uh oh!

lamblin Jan 28, 2016

Choose a reason for hiding this comment

Uh oh!

lamblin commented Jan 28, 2016

Uh oh!

nouiz commented Jan 29, 2016

Uh oh!

nouiz commented Feb 1, 2016

Uh oh!

nouiz Feb 1, 2016

Choose a reason for hiding this comment

Uh oh!

gyom Feb 1, 2016

Choose a reason for hiding this comment

Uh oh!

nouiz commented Feb 1, 2016

Uh oh!

gyom commented Feb 1, 2016

Uh oh!

nouiz commented Feb 1, 2016

Uh oh!

lamblin Feb 3, 2016

Choose a reason for hiding this comment

Uh oh!

nouiz Feb 3, 2016

Choose a reason for hiding this comment

Uh oh!

lamblin commented Feb 3, 2016

Uh oh!

abergeron commented Feb 3, 2016

Uh oh!

nouiz commented Feb 3, 2016

Uh oh!

gyom commented Feb 3, 2016

Uh oh!

nouiz commented Feb 3, 2016

Uh oh!

nouiz commented Feb 8, 2016

Uh oh!

gyom commented Feb 9, 2016

Uh oh!

nouiz commented Feb 9, 2016

Uh oh!

Uh oh!