search

bartvm / nmt

Neural machine translation
MIT License
2 stars 2 forks source link

Code cleanup #12

Closed bartvm closed 8 years ago

bartvm commented 8 years ago
JinseokNam commented 8 years ago

Can you point me to some nice tutorials for setting up TRAVIS?

bartvm commented 8 years ago

Have a look at https://github.com/mila-udem/blocks/blob/master/.travis.yml for the conda and MKL installation if you want. I need to link it to Travis first though, I think I should be able to do that for free even though it's a private repo as a student, but let me have a look.

bartvm commented 8 years ago

Okay, should be linked and enabled as soon as the .travis.yml file is committed.

JinseokNam commented 8 years ago

It looks like Travis works now, but I've just committed a very simple travis.yml.

bartvm commented 8 years ago

I keep coming across truly horrendous in the code (model_options = locals().copy() is possibly even worse than from optimizers import *; eval(optimizer)(args). I'll have a look at that now.

JinseokNam commented 8 years ago

I'm trying to create a new branch from the current master, then apply part of the things that I've done in this branch 4ee1df0. Your comment about locals().copy() will be also considered.

anirudh9119 commented 8 years ago

Thank you! I'll keep that in mind.

bartvm commented 8 years ago

@JinseokNam Don't worry about locals().copy() for now; I started a refactoring of the way we specify experiments (using YAML files instead of editing the source directly) that should fix that too, but might take till tomorrow for me to make a PR.

If you have anything you want to port from those commits, please do! But keep the PRs as small as possible i.e. it's fine to make a PR that only changes 1 or 2 lines and make 10 PRs a day; that's often far more efficient than making 1 big PR.

JinseokNam commented 8 years ago

I think the problems raised in this issue have been solved, so I close this.