-
Notifications
You must be signed in to change notification settings - Fork 739
Add conv-tasnet training script #896
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
Conversation
25403d0 to
1f7df3c
Compare
fmassa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a quick looks and overall the structure looks pretty good!
I've made some comments about a few things I've done differently in my past scripts, but they are by no means meant to be the "ground-truth" on how to do things, and I'd be happy to hear your thoughts as well.
152db73 to
b970f1c
Compare
c3bd902 to
058620a
Compare
|
|
||
| </details> | ||
|
|
||
| ### SLURM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
submitit is great for user experience: https://github.com/facebookresearch/detr/blob/master/run_with_submitit.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E.g. can do something like
python run_with_submitit.py --nodes 4 --timeout 3000 --coco_path /datasets01/COCO/022719
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://github.com/facebookincubator/submitit for submitit repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at it, but it feels like it adds the more learning curve to go through this example.
The code is right now completely agnostic to SLURM yet runnable with SLURM.
I think that leaving the decision of the adaptation of submitit to users will keep our code simpler.
As long as the bare minimum of how to use in SLURM environment is explained in README, then it should do.
|
|
||
| The following is the evaluation result of training the model on WSJ0-2mix and WSJ0-3mix datasets. | ||
|
|
||
| ### wsj0-mix 2speakers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to upload a model to make it easier to reproduce these numbers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that will be great.
c10bf1b to
c97b349
Compare
203cc7c to
2a9714d
Compare
examples/source_separation/README.md
Outdated
|
|
||
| ### Overview | ||
|
|
||
| To traing a model, you can use [`train.py`](./train.py). This script takes the form of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: train
2a9714d to
1f9fbfb
Compare
|
Thanks! |
remove experimental from rpc tutorial
fix the error of generated code in subgraph rewrite
#897