jbornschein / draw

Reimplementation of DRAW
MIT License
347 stars 84 forks source link

Alternative dataset: sketches + start train-draw from an existing model parameters #5

Closed udibr closed 9 years ago

udibr commented 9 years ago

Perhaps you are interested. I am trying to generate human sketches of objects (see datasets/README.md)

In any case I have added an option to train-draw called oldmodel which lets continue a training run using the parameters value of the oldmodel as a starting point

udibr commented 9 years ago

I just added a bug fix in sample.py. It ignored the seed_default in config (because it was loded from the saved model) as a result you allways got the same images.

I am not an export in git pull request so I am not sure if you see this change or not

udibr commented 9 years ago

small correction to the seed reset

jbornschein commented 9 years ago

Thank you for the pull request! I will definitely merge some of your changes. E.g. the seed fit.

I'm not yet sure if this repository should include further experiments/applications of DRAW to different datasets. While it would be a much more valuable resource if it would include a broad set of experimental results, I'm also afraid this will harm maintainability. Especially because Blocks is still very much a "moving target" and we can not simply perform automated tests for these experiments because it would just take to long to run them in e.g. travis.

What do you think? Should this repo. include experiments on datasets not described in the original DRAW paper? Even when we have to assume that some of them might be broken because we, the maintainer cannot regularly validate them with all the changes going on?

ghost commented 9 years ago

YES DEFINITELY !!

I'm happy to contribute experiments on simpler datasets, which should train faster and be more transparent, such as the online hand writing experiments, Alex Graves used in his earlier paper.

I've done some work to prepare the data

I'm currently working on trying to implement these with my Torch DRAW implementation, but as soon as I get it working, I'll try to validate my results with this implementation.

udibr commented 9 years ago

Thanks for considering this pull 1) There are 3 lines in sample.py (86-87) which reset the default_seed when generating images. I was very confused when I wanted to get a different set of generated images and always got the same results. I opened an issue on Blocks for having a proper way of doing this reset (moving target indeed!) I suggest you do merge these 3 lines.

2) I agree there is great beauty in keeping things simple. However, it already looks like your project is acting as a focus point for people who want to explore the DRAW algorithm (sadly deepmind did not publish their Torch code.) My attempt was to modify your original code and everything in the original directory as little as possible. This is why I have placed a separate README file for the sketch data set and placed all the related code in a new directory. However I had to have some kind of flag that will redirect train-draw.py to work on the different dataset. Even if the new code breaks, as long as a user does not set name=sketch then there will be no ill effect.

udibr commented 9 years ago

3) I also opened a pull request for "fuel" project to include the binarized sketch data. For this is on hold because they are refactoring, but when done, I will cleanup this project. However, there will be few leftovers such as the separate README file and the switch inside train-draw to select the correct data source (hopefully fuel will supply some generic mechanisem/template to handle this.)

udibr commented 9 years ago

I got a response from bartvm about the reset problem and it made me realized that my hack worked by accident. The real problem is that sample is a method of draw which is the real cause of the problem. The sample method is called only from sample.py and not from train-draw.py so you would expect its Theano graph to be new in sample.py however the sample method is part of the DrawModel object which was already created in train-draw and it includes (through inheritance from Random) all the random seed mechanisem which is already preloaded with the default_seed used at the time when it was created.

I think a better solution will to remove this method and put it in a seperate wrapper object. This object will be created in sample.py (with a new seed) and in its init it will wrap the old DrawModel object (with the old seed)

If you want, I can code it, but since this goes deep into draw.py then perhaps you want to do it... let me know (in any case the hack works for now)

jbornschein commented 9 years ago

Sorry for letting this PR sit around here for so long. I merged it as it was.

I don't think we should remove the sampling code from the draw model -- especially because I wanted to draw samples and display them during learning once some infrastructure for that landed in blocks (see https://github.com/bartvm/blocks/pull/447).

Your method of reseeding/resetting the RNG does not look very 'elegant' -- but I think it's the method we should prefer for now.

Thanks!