-
Notifications
You must be signed in to change notification settings - Fork 27.3k
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
[Wav2vec2 + LM Test] Improve wav2vec2 with lm tests and make torch version dependent for now #18749
[Wav2vec2 + LM Test] Improve wav2vec2 with lm tests and make torch version dependent for now #18749
Conversation
…to improve_wav2vec2_with_lm_tests
It fails consistently since PT 1.12. |
The documentation is not available anymore as the PR was closed or merged. |
Note after some more debugging the reason seems to be in ds = load_dataset("common_voice", "en", split="train", streaming=True)
ds = ds.cast_column("audio", datasets.Audio(sampling_rate=16_000))
ds_iter = iter(ds)
sample = next(ds_iter)
print(sample["audio"]["array"]) yields different results between |
tests/models/wav2vec2_with_lm/test_processor_wav2vec2_with_lm.py
Outdated
Show resolved
Hide resolved
tests/models/wav2vec2_with_lm/test_processor_wav2vec2_with_lm.py
Outdated
Show resolved
Hide resolved
@ydshieh good for a second review |
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.
Thank you @patrickvonplaten . LGTM, but probably a small change to make it better.
|
||
# TODO(Patrick): This if-else version statement should be removed once | ||
# https://github.com/huggingface/datasets/issues/4889 is resolved | ||
if version.parse(torch.__version__) >= version.parse("1.12.0"): |
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.
Maybe let's use what has been done in
#18460
?
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.
Yes definitely - thanks!
tests/models/wav2vec2_with_lm/test_processor_wav2vec2_with_lm.py
Outdated
Show resolved
Hide resolved
…rsion dependent for now (huggingface#18749) * add first generation tutorial * remove generation * make version dependent expected values * Apply suggestions from code review * Update tests/models/wav2vec2_with_lm/test_processor_wav2vec2_with_lm.py * fix typo
Trying to correct potentially flaky test:
The test actually always passed for me locally.
I've now changed list to tensor as list comparison with float numbers seems brittle. @ydshieh - just to better understand, is this test consistently failing or was it flaky?