-
Notifications
You must be signed in to change notification settings - Fork 662
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
Refactor LibriSpeech Conformer RNN-T recipe #2366
Refactor LibriSpeech Conformer RNN-T recipe #2366
Conversation
trainer.fit(model) | ||
model = ConformerRNNTModule(str(args.sp_model_path)) | ||
data_module = get_data_module(str(args.librispeech_path), str(args.global_stats_path), str(args.sp_model_path)) | ||
trainer.fit(model, data_module) |
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.
It'll be great if we add a --resume-checkpoint
argument with None
as default value, and change this line to
trainer.fit(model, data_module, ckpt_path=args.resume_checkpoint)
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.
--resume-from-checkpoint may sound better
0ec7ce4
to
0078c68
Compare
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.
Stamp
from pytorch_lightning import LightningDataModule, seed_everything | ||
|
||
|
||
seed_everything(1) |
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.
Is this the right place to seed? I would imagine this happen once (and only once) at the very beginning of the CLI entry point.
seed_everything(1) | ||
|
||
|
||
def _batch_by_token_count(idx_target_lengths, token_limit, sample_limit=None): |
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.
Can you change sample_limit to batch_size and token_limit to max_tokens per our previous discussion? It's ok if you want to do that in another PR.
return dataloader | ||
|
||
def test_dataloader(self): | ||
dataset = torchaudio.datasets.LIBRISPEECH(self.librispeech_path, url="test-clean") |
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.
better make the eval split customizable as we discussed. again I can do that in a later PR as well.
train_transform, | ||
val_transform, | ||
test_transform, | ||
max_token_limit=700, |
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.
max_tokens should be fine. "max" duplicates with "limit"
val_transform, | ||
test_transform, | ||
max_token_limit=700, | ||
sample_limit=2, |
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.
batch_size
@hwangjeff has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Hey @hwangjeff. |
Modifies the example LibriSpeech Conformer RNN-T recipe as follows:
partial
equivalents (resolves pickling issues in certain runtime environments).