Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove machine translation and add a link to an external repo #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rizar
Copy link
Contributor

@rizar rizar commented Nov 23, 2015

Let us start the procedure of moving the machine translation code out of Blocks-examples. @orhanf , would you like to create a new repository for it? Or maybe I should create a new repository under @mila-udem? What do you guys think, @bartvm and @dwf?

@rizar rizar changed the title Remove machine translation and a link to an external repo Remove machine translation and add a link to an external repo Nov 23, 2015
@nouiz
Copy link

nouiz commented Nov 23, 2015

What are the motivation to move this outside of blocks-examples?

On Mon, Nov 23, 2015 at 10:31 AM, Dzmitry Bahdanau <notifications@github.com

wrote:

Let us start the procedure of moving the machine translation code out of
Blocks-examples. @orhanf https://github.com/orhanf , would you like to
create a new repository for it? Or maybe I should create a new repository
under @mila-udem https://github.com/mila-udem? What do you guys think,

@bartvm https://github.com/bartvm and @dwf https://github.com/dwf?

You can view, comment on, or merge this pull request online at:

#63
Commit Summary

  • Remove machine translation and a link to an external repo

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#63.

@rizar
Copy link
Contributor Author

rizar commented Nov 23, 2015

Because Blocks-examples and NMT demo have quite different purposes.

Blocks-examples is supposed to contain minimalistic toy examples of using Blocks. We would like to keep it small, especially in terms of the maximum size of an example. We are committed to keep all examples fully functional for the cutting-edge Blocks, which means that for every Blocks PR necessary changes have to be applied here.

NMT demo, on the other hand, is a fully-functional machine translation implementation. It's purpose is to reproduce results from a few machine translation papers. It has a lot of code dealing with real-world issues specific to machine translation and also attracts a broad audience that does not want to learn anything about Blocks or even Theano, but just wants to train a machine translation model.

Trying to sum it up, I don't think that Blocks-examples should contain real-world examples with all their overwhelming complexity, because this is too much of a load for Blocks maintainers.

@orhanf
Copy link
Contributor

orhanf commented Nov 23, 2015

@rizar to double check, according to b29e1fc, i'm moving it under https://github.com/orhanf/neural-MT , is that good or will be moved to another repo?

@rizar
Copy link
Contributor Author

rizar commented Nov 23, 2015

This URL is just a placeholder to start the discussion. Maybe this should
be @mila-udem repository, I don't know yet.

On 23 November 2015 at 11:32, Orhan Firat notifications@github.com wrote:

@rizar https://github.com/rizar to double check, according to b29e1fc
b29e1fc,
i'm moving it under https://github.com/orhanf/neural-MT , is that good or
will be moved to another repo?


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

@@ -52,3 +52,4 @@ Blocks:
* `Character-level RNN <https://github.com/johnarevalo/blocks-char-rnn>`_
* `DRAW model <https://github.com/jbornschein/draw>`_
* `Speech recognition <https://github.com/rizar/attention-lvcsr>`_
* `Machine Translation <https://github.com/orhanf/neural-MT>`_
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe don't capitalize the T, to be consistent with "speech recognition" above.

Otherwise,, everything LGTM.

@dwf
Copy link
Contributor

dwf commented Nov 23, 2015

Looks like we will need the fix from mila-iqia/blocks@e185f3d for the buildbot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants