Fix WhisperForConditionalGeneration to respect generation_config?

[mizoru]

I had to change a line in WhisperGenerationMixin for it to generate with timestamps during evaluation. Many parameters of WhisperGenerationMixin.generate are None by deafult. So that when WhisperForConditionalGeneration.generate is called with default parameters and return_timestamps is set to True in generation_config, WhisperGenerationMixin._set_return_timestamps is called and replaces the value in the config with None. The same thing happens with return_token_timestamps in the method _set_return_outputs, so I'm unsure if this is intended. Should I maybe open an issue first?

What does this PR do?

Change WhisperGenerationMixin._set_return_timestamps to only replace the value return_timestamps in generation_config if new value is not None.

Before submitting

• [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
• [x] Did you read the contributor guideline, Pull Request section?
• [ ] Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
• [ ] Did you make sure to update the documentation with your changes? Here are the documentation guidelines, and here are tips on formatting docstrings.
• [ ] Did you write any new necessary tests?

Who can review?

I can see @patrickvonplaten was the last person who worked on these lines.

[amyeroberts]

cc @sanchit-gandhi @kamilakesbi

[mizoru]

Sorry, I'm a bit in over my head on how to contribute this properly right now, but:

This should work

    @staticmethod
    def _set_return_timestamps(return_timestamps, is_shortform, generation_config):
        if return_timestamps is None and hasattr(generation_config, "return_timestamps"):
            return_timestamps = generation_config.return_timestamps
        if not is_shortform:
            if return_timestamps is False:
                raise ValueError(
                    "You have passed more than 3000 mel input features (> 30 seconds) which automatically enables long-form generation which "
                    "requires the model to predict timestamp tokens. Please either pass `return_timestamps=True` or make sure to pass no more than 3000 mel input features."
                )

            logger.info("Setting `return_timestamps=True` for long-form generation.")
            return_timestamps = True

        if return_timestamps and not hasattr(generation_config, "no_timestamps_token_id"):
            raise ValueError(
                "You are trying to return timestamps, but the generation config is not properly set. "
                "Make sure to initialize the generation config with the correct attributes that are needed such as `no_timestamps_token_id`. "
                "For more details on how to generate the approtiate config, refer to https://github.com/huggingface/transformers/issues/21878#issuecomment-1451902363"
            )
        generation_config.return_timestamps = return_timestamps

[kamilakesbi]

Hi @mizoru,

Thanks for iterating on this! Could you please open an issue with a min reproducer of the error you get before making these changes?

[HuggingFaceDocBuilderDev]

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.

[kamilakesbi]

It will be solved by PR #31296 :)