🧼 remove v4.44 deprecations#34245
Conversation
src/transformers/modeling_utils.py
Outdated
There was a problem hiding this comment.
(followed this instruction whenever shard_checkpoint was used)
There was a problem hiding this comment.
Unfortunately, this is not the exact function. We can't just replace it directly. You need to build the shard and the index again using the output of this function. Check the example here: https://huggingface.co/docs/huggingface_hub/main/en/package_reference/serialization#huggingface_hub.split_torch_state_dict_into_shards.example
There was a problem hiding this comment.
Thanks for fixing but the processing logic refactor wen longer than I expected. We are still doing the last bits in #33424 and the checkpoints on the hub are not updated yet (tracker #33374). So we can't be raising ValueError yet
Maybe we can change the deprecation version to be after a few more minor releases. We have also same warnings in all llava and blip models afair. Also now I think we can even do more than just a few releases, since the changes will invalidate many models on the hub that are not owned by us. WDYT, which version we can put in the message?
We should then support it for as long as possible :) If it is critical to any popular model, I would not even set a deprecation version target at all. Leave the exception without target, and set a target when it blocks you from doing something OR if it is causing many issues. An example of this is supporting If it is only used in some niche models, and applying the deprecation (raising exception) is a significant win for transformers, then I would set at least 6 minor versions. LMK which one is it, and I will update the warning. |
|
I hope you meant I would say this particular one is a niche model, given download stats from the hub. And most older llava and BLIP models may be outdated fast as we get newer and better releases each month. But I think we still can't enforce users to do something by elevating this to an Exception. My preference is to keep it for 6 minor versions as a warning, and try to update official models on the hub until that. We have write access to all of them afaik except for Video-LLaVA so updating will be straightforward |
src/transformers/modeling_utils.py
Outdated
There was a problem hiding this comment.
Unfortunately, this is not the exact function. We can't just replace it directly. You need to build the shard and the index again using the output of this function. Check the example here: https://huggingface.co/docs/huggingface_hub/main/en/package_reference/serialization#huggingface_hub.split_torch_state_dict_into_shards.example
@zucchini-nlp haha yes, |
dcd2588 to
48f5982
Compare
|
@zucchini-nlp @SunMarc PR comments addressed, LMK if they look good to you :) |
|
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. |
zucchini-nlp
left a comment
There was a problem hiding this comment.
Thanks, looks good to me! Can we propagate the same thing to all files with this deprecation warning? I guess it is all LLaVA and some of BLIP models (processing and modeling files)
|
@zucchini-nlp done ✅ |
SunMarc
left a comment
There was a problem hiding this comment.
Nice ! Thanks for iterating !
LysandreJik
left a comment
There was a problem hiding this comment.
Ok, sounds good! Thanks for the PR and cleanup @gante
|
To pass the CI, you need to rebase the PR @gante ;) |
* remove v4.44 deprecations * PR comments * deprecations scheduled for v4.50 * hub version update * make fiuxp --------- Co-authored-by: Marc Sun <57196510+SunMarc@users.noreply.github.com> Co-authored-by: Arthur <48595927+ArthurZucker@users.noreply.github.com>
What does this PR do?
See title :)
@zucchini-nlp -> video llava changes
@SunMarc -> all other changes