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

disable speed perturbation by default #1176

Merged
merged 5 commits into from
Aug 10, 2023

Conversation

JinZr
Copy link
Collaborator

@JinZr JinZr commented Jul 20, 2023

No description provided.

@danpovey
Copy link
Collaborator

We should probably do this, even though it may make some of the recipes not fully reproducible (i.e. it would require to change the learning rate schedules and num-epochs).
It might make sense to actually change the LR schedules and epochs at the same time. This would not be fully
foolproof, since some people will have previously-dumped non-perturbed features on disk. But it will ensure that
from a fresh checkout things would be basically reproducible.
(if README files have example usages, we could perhaps just write the alternative command lines that you would use
if the features were not dumped with speed perturbation, with the num-epochs and if applicable lr-epochs multiplied by 3.)

@pzelasko
Copy link
Collaborator

It doesn’t help anymore?

@JinZr
Copy link
Collaborator Author

JinZr commented Jul 21, 2023 via email

@pkufool
Copy link
Collaborator

pkufool commented Jul 21, 2023

It doesn’t help anymore?

We did experiments on librispeech dataset, the performance of speed-pertur is the same as no speed-pertur (3 times epochs).

@yfyeung
Copy link
Collaborator

yfyeung commented Jul 23, 2023

We did experiments on librispeech dataset, the performance of speed-perturb is the same as no speed-perturb (3 times epochs).

Somehow slightly worse than with perturb

Copy link
Collaborator

@yfyeung yfyeung left a comment

Choose a reason for hiding this comment

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

Maybe you can use:

if speed_perturb and "train" in partition:

@JinZr
Copy link
Collaborator Author

JinZr commented Jul 24, 2023 via email

@JinZr
Copy link
Collaborator Author

JinZr commented Jul 27, 2023 via email

@danpovey
Copy link
Collaborator

danpovey commented Jul 27, 2023 via email

Copy link
Collaborator

@csukuangfj csukuangfj left a comment

Choose a reason for hiding this comment

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

Thanks! Left a minor comment. Otherwise, it looks good to me.

1. changed the naming scheme from `speed-perturb` to `perturb-speed` to align with the librispeech recipe

>> https://github.com/k2-fsa/icefall/blob/00256a766921dd34a267012b0e2b8ff7d538f0e6/egs/librispeech/ASR/local/compute_fbank_librispeech.py#L65

2. changed arg type for `perturb-speed` to str2bool
Copy link
Collaborator

@csukuangfj csukuangfj left a comment

Choose a reason for hiding this comment

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

Thanks!

@csukuangfj csukuangfj merged commit 74806b7 into k2-fsa:master Aug 10, 2023
3 checks passed
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.

6 participants