Skip to content
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

Config: allow saving with generative parameters if untouched #33886

Closed
wants to merge 14 commits into from

Conversation

gante
Copy link
Member

@gante gante commented Oct 2, 2024

What does this PR do?

⚠️ ⚠️ ⚠️ don't review yet, sorting all tests and use cases is taking more time than I thought 👀


In a recent PR (#32659), we started raising exceptions if we were to store a model config with generative parameters. If we were saving a model, which saves the config, the generative parameters were moved to GenerationConfig before saving the PreTrainedConfig, working around the issue when saving a model.

However, a common use case was uncovered. If a user loads a PreTrainedConfig with generative parameters and tries to save it, the user has done nothing wrong and yet they see an exception. This PR lowers the exception to a warning in that situation, preventing the code from crashing :)

(Thank you @regisss for flagging this issue and @ydshieh for the testing suggestion)

@gante gante requested a review from LysandreJik October 2, 2024 13:17
Copy link
Contributor

@regisss regisss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing it Joao!

@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.

@gante gante force-pushed the save_whisper_config branch from 98bf416 to d8aec4d Compare October 2, 2024 15:23
@gante gante force-pushed the save_whisper_config branch from d8aec4d to 29e63cb Compare October 2, 2024 16:08
@gante gante added the WIP Label your PR/Issue with WIP for some long outstanding Issues/PRs that are work in progress label Oct 2, 2024
@gante
Copy link
Member Author

gante commented Oct 2, 2024

This approach is opening a can of worms 😠

Going to fix this issue another way

@gante gante closed this Oct 2, 2024
@gante gante deleted the save_whisper_config branch October 2, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Label your PR/Issue with WIP for some long outstanding Issues/PRs that are work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants