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

Generate: inner decoding methods are no longer public #29437

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

gante
Copy link
Member

@gante gante commented Mar 4, 2024

What does this PR do?

This PR sets the inner decoding methods (such as greedy_search) as private methods, with a deprecation warning.

This will enable us to complete the following projects with fewer speed bumps:

  1. torch.compile: as seen in the PoC PR, a few interface pieces should be touched. E.g. Some special tokens can be a list (e.g. eos_token_id), which is not compile-friendly, and thus torch.tensor should be used instead;
  2. Separate prefill function. This will significantly change the interface to the inner decoding methods;
  3. More flexible, composable generate. Having multiple interfaces to maintain will constrain our ability to refactor the code.

@gante gante requested a review from ArthurZucker March 4, 2024 12:46
@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.

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Thanks, might be easier to have both contrastive_search and _ contrastive_search for overloading? Tho it gives freedom, up to you!

Comment on lines +1806 to +1809
logger.warning_once(
"Calling `contrastive_search` directly is deprecated and will be removed in v4.41. Use `generate` or a "
"custom generation loop instead.",
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

we could even leave it as is, this way custom self._contrastive_search are easily changeable for users? or something like that

Copy link
Member Author

Choose a reason for hiding this comment

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

In the future, I don't want to have all these methods, as they are near copies of each other -- e.g. greedy_search and sample differ by <5 lines. As such, some of them will be deleted, so users won't be able to call e.g. _greedy_search :D

@gante gante merged commit 87a0783 into huggingface:main Mar 5, 2024
22 checks passed
@gante gante deleted the deprecate_decoding_methods branch March 5, 2024 10:27
@gante gante mentioned this pull request Mar 14, 2024
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants