fix: [whisper] don't overwrite GenerationConfig's return_timestamps when return_timestamps is not passed to generate function
#31296
Conversation
|
Possibly cc @gante as it relates to generation |
|
cc @kamilakesbi |
kamilakesbi
left a comment
There was a problem hiding this comment.
Hi @hrl,
Thanks for working on this! it LGTM :)
@amyeroberts we should be able to merge
amyeroberts
left a comment
There was a problem hiding this comment.
Thanks for handling this!
Could you add a test?
I'd just like to confirm this behaviour is consistent with the rest of the generation API cc @gante
|
@hrl did you have time to iterate on this ? thank you! |
@kamilakesbi conflicts resolved.
@amyeroberts I'm not familiar with the rest of the generation API. Should I write a test for this? The behavior will be different only when |
Oh, no worries, this was a question for @gante, our generate king and who knows all the ins-and-outs best :) |
|
Gentle ping @gante |
gante
left a comment
There was a problem hiding this comment.
Apologies for the delayed review, and thank you for fixing 🙏
|
(@amyeroberts @kamilakesbi the root cause for the issue behind this PR is that Whisper's |
… when `return_timestamps` is not passed to `generate` function (huggingface#31296) [whisper] don't overwrite return_timestamps when not passed to generate
What does this PR do?
When
WhisperGenerationMixin.generatecalled withoutreturn_timestamps, GenerationConfig'sreturn_timestampswill be overwritten toNone.It's okay if we can control
generate's arguments. But in some particular case, for example, inTrainer.evaluation_loop, then inSeq2SeqTrainer.prediction_step, there is no way to passgen_kwargstogeneratefunction. (#19777)Which makes it impossible to generate with timestamps to calculate generative metrics when training.
This PR fixes this case by initializing
return_timestampsto GenerationConfig's value.Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@patrickvonplaten @sanchit-gandhi