-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
fix Parameter dtype in audio models #30310
Conversation
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
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.
LGTM
Let me check CI |
The original failed pipeline test also passes. I will run a whole pipeline test and let's see. For model testing, since you checked every model modified, I also think it's fine and don't see what else should be checked manually. |
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.
LGTM - thanks @ylacombe! Fine for me to keep the .uniform()
in the init for simplicity as well
all good! merge now, thanks a lot. |
What does this PR do?
tests/pipelines/test_pipelines_automatic_speech_recognition.py::AutomaticSpeechRecognitionPipelineTests::test_wav2vec2_conformer_float16
was failing becausemasked_spec_embed
is initialized as afp32
tensor.transformers/src/transformers/models/wav2vec2_conformer/modeling_wav2vec2_conformer.py
Line 1238 in 28a2283
I thus changed the tensor from
FloatTensor
toTensor
. Ideally, I would have moved the.uniform_
, to the_init_weights
, but turns thatmodel.apply
doesn't iterate onParameter
.Slow tests on every modeling tests of the files I've changed all passed, but I might have missed some other tests to run, any ideas @ydshieh ?
cc @ydshieh because of the failing test and @ArthurZucker because we've talked about it