-
Notifications
You must be signed in to change notification settings - Fork 33.9k
Fix init weights in remote code #43768
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
bc132eb
init or tie weight in remote code
zucchini-nlp b05a477
processing
zucchini-nlp 9726c6f
config attr
zucchini-nlp 0718e59
maybe? the special token logic is breaking many tests
zucchini-nlp c8e77c1
updates
zucchini-nlp 2cf6aa9
Merge branch 'main' into vllm-v5-bump
zucchini-nlp e54a6fb
oh c'mon
zucchini-nlp 195a552
omg
zucchini-nlp 9dccede
try None and see if tests fail
zucchini-nlp 1db3144
oops
zucchini-nlp dd6296c
Merge branch 'main' into vllm-v5-bump
zucchini-nlp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
module's never have an
_is_hf_initializedattr, ig this is a typo? Otherwise it causes the whole model to be random init when remote code has an old-format_init_weightsdefined and it takes ages for big modelsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right they never do now, only the tensors have them.
The check should only run for remote code I think no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, for local models it will not have much effect because we check
if weight._is_hf_initializedlater in theinitialization.py. So we are never random init weights for already loaded paramsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, what is this check? Any module with both a weight and something else would be skipped if the "something else" is a missing param!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modules WILL have the flag when weight init is not called from
from_pretrained. Image doing raw instantiation with a composite model, such asmodel = ModelArch(config), then every submodel will runpost_initand initialize its weights, and set the flag, so that next post_init does not run it again!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was due to remote code models re-init weights randomly. For ex, this one has a custom
_init_weightsand will random init without checking if weights were loaded from ckpt or not. Or do we not support this type of models on purpose in v5 in which case we can revert and skip these models in vLLM?https://huggingface.co/TIGER-Lab/VLM2Vec-Full/blob/main/modeling_phi3_v.py#L1237-L1247
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v5 broke most of the loading of remote code. For this model in particular, note how e.g. the rope module would have random weights anyway as the non-persistent buffer is not reinitialized explicitly and so would lose its value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh actually they initialize it at None and then update it, so would be fine in this case