-
Notifications
You must be signed in to change notification settings - Fork 30.7k
fix asr ut failures #41332
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 asr ut failures #41332
Conversation
Signed-off-by: Yao, Matrix <[email protected]>
vocab_tensor = torch.arange(scores.shape[-1], device=scores.device) | ||
suppress_token_mask = isin_mps_friendly(vocab_tensor, self.suppress_tokens) | ||
suppress_token_mask = isin_mps_friendly(vocab_tensor, self.suppress_tokens.to(scores.device)) | ||
scores = torch.where(suppress_token_mask, -float("inf"), scores) |
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.
In multi-device cases(like put 2 devices to run):
in current implementation, in assistant decoding case, assistant model will reuse main model's SuppressTokensLogitsProcessor
, which place the suppress_tokens
in the same device as input_tensor
(which is device 0
). assistant model will ingest encoder_outputs
of the main model and do the decoder(in whisper case), while encoder_outputs
may in device 1
but main model's suppress_tokens
which is main model's is in device 0
, so lead to RuntimeError
:
RuntimeError: Expected all tensors to be on the same device, but got test_elements is on xpu:0, different from other tensors on xpu:1 (when checking argument in method wrapper_XPU_isin_Tensor_Tensor)
So based on current implementation(that assistant model shares main model's SuppressTokensLogitsProcessor
), I move suppress_tokens
to scores.device
while doing isin
.
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 for fixing ! cc @eustlb for final checks
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! thanks for your help 🤗
transcription_ass = pipe(sample.clone().detach(), generate_kwargs={"assistant_model": assistant_model})["text"] | ||
transcription_non_ass = pipe(sample)["text"] |
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 for catching the incorrect inversion here!
4 tests failed with AttributeError: 'AudioDecoder' object has no attribute 'copy':
pytest -rA tests/pipelines/test_pipelines_automatic_speech_recognition.py::AutomaticSpeechRecognitionPipelineTests::test_speculative_decoding_whisper_non_distil pytest -rA tests/test_pipeline_mixin.py::AutomaticSpeechRecognitionPipelineTests::test_speculative_decoding_whisper_non_distil pytest -rA tests/pipelines/test_pipelines_automatic_speech_recognition.py::AutomaticSpeechRecognitionPipelineTests::test_whisper_prompted pytest -rA tests/test_pipeline_mixin.py::AutomaticSpeechRecognitionPipelineTests::test_whisper_prompted
after fixing, they all passed
@SunMarc @ydshieh , pls help review, thx very much.