Generate: Deprecate returning legacy cache by default; Handle use_cache=False
#32863
Conversation
|
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. |
use_cache=False
There was a problem hiding this comment.
moved up to this documentation section (Parameters that control the generation strategy used), which makes more sense
There was a problem hiding this comment.
new cache-related docs section in GenerationConfig, moved all cache-related flags here
src/transformers/generation/utils.py
Outdated
There was a problem hiding this comment.
In our docs we often mention that there are two ways to parameterize generate (generation_config or pass arg to generate). I don't think we need to be verbose here.
Also, setting through config is deprecated 😉
src/transformers/generation/utils.py
Outdated
There was a problem hiding this comment.
rewrote this one.
The old description was outdated (legacy cache), and we now know that different models have different caches, so we shouldn't be precise here. The model class docs can be more precise, let's redirect users there.
src/transformers/generation/utils.py
Outdated
There was a problem hiding this comment.
(These aliases made sense in the past, not anymore. They are, however, hard to deprecate!)
src/transformers/generation/utils.py
Outdated
There was a problem hiding this comment.
New function, moving the cache logic from generate. I've organized the logic in blocks, putting the cases where we DON'T prepare a new cache at the top.
It is doing essentially the same, except for the Quick escape route 2, which is new. Added the warning in Quick escape route 3.
src/transformers/generation/utils.py
Outdated
There was a problem hiding this comment.
This logic is expanded to handle a deprecation cycle
tests/generation/test_utils.py
Outdated
There was a problem hiding this comment.
changes in this file: pass use_cache to generate, instead of relying on model.config.use_cache=False and its side-effects
added a check to confirm that the cache is None when we pass use_cache=False
ArthurZucker
left a comment
There was a problem hiding this comment.
Cool! Let's make sure slow tests all pass as well here!
src/transformers/generation/utils.py
Outdated
There was a problem hiding this comment.
let's create an issue and leave it up to the community in the mean time!
a442522 to
e8492bf
Compare
What does this PR do?
Another step towards using
Cacheeverywhere 💪This PR makes the following [
Cache+generate]-related changes:use_cache=False(fixes Cache updating when use_cache = False #32843 )generatetests now explicitly passuse_cache, instead of setting it inmodel.config🤢 We were relying on a LOT of side effects, and missing the incorrect case mentioned in Cache updating when use_cache = False #32843generation_configCacheinstance by default ongenerategenerateinto a single function, and reorganize the logic by blocks