Skip to content

Conversation

@yonigozlan
Copy link
Member

Not necessary anymore, and was crashing the model.

Fixes #42837

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

Comment on lines -1979 to -1983
_tied_weights_keys = {
"prompt_encoder.shared_embedding.positional_embedding": "shared_image_embedding.positional_embedding"
}
# need to be ignored, as it's a buffer and will not be correctly detected as tied weight
_keys_to_ignore_on_load_missing = ["prompt_encoder.shared_embedding.positional_embedding"]
Copy link
Member

Choose a reason for hiding this comment

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

Is it really not needed at all anymore? We could otherwise very easily allow buffers to be tied (but a bit weird)

Copy link
Member Author

Choose a reason for hiding this comment

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

Everything seems to work just fine without it 🤷, no warnings either

Copy link
Member

Choose a reason for hiding this comment

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

Well, for sure you won't have warnings about tied weights if we never tie them haha - did you make sure that both weights correctly live inside ALL main checkpoints? If it's the case, then yes probably they were never supposed to be tied

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes all is working fine, the tying was also removed for sam/sam2, we just forgot about this one I guess

@github-actions
Copy link
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: edgetam_video, sam2_video, sam3_tracker_video

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.

Regression in SAM2 caused by changes in mark_tied_weights_as_initialized

3 participants