-
Notifications
You must be signed in to change notification settings - Fork 223
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
add user defined kaldi feature type #1101
add user defined kaldi feature type #1101
Conversation
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.
Thanks, LGTM!
Seems like the assertion failed at https://github.com/lhotse-speech/lhotse/blob/master/test/test_kaldi_dirs.py#L213 |
You’ll need to update the jsonl files used in tests to have the new feature type. |
Head branch was pushed to by a user without write access
Changed the type in features.jsonl.gz from kaldi_native_io to kaldi-fbank |
lhotse/kaldi.py
Outdated
@@ -80,6 +80,7 @@ def load_kaldi_data_dir( | |||
map_string_to_underscores: Optional[str] = None, | |||
use_reco2dur: bool = True, | |||
num_jobs: int = 1, | |||
feature_type: str = 'kaldi-fbank' |
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.
You also need to update
lhotse/lhotse/bin/modes/kaldi.py
Lines 35 to 41 in cb0b7d3
@click.option( | |
"-j", | |
"--num-jobs", | |
default=1, | |
type=int, | |
help="Number of jobs for computing recording durations.", | |
) |
You can add another option for feature_type
.
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.
updated
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.
You forgot to update the subsequent python function.
You have to add it as an argument for the function that follows.
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.
Sorry for my carelessness, see if any further modifications need to be done.
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.
Thanks, LGTM! Please just fix the remaining style checks.
Head branch was pushed to by a user without write access
Add user-defined kaldi feature type, by default is 'kaldi-fbank'