Skip to content

Fix self.dropout_p is not defined for SamAttention/Sam2Attention#40667

Merged
Cyrilvallez merged 2 commits intohuggingface:mainfrom
yonigozlan:fix-training-sam
Sep 4, 2025
Merged

Fix self.dropout_p is not defined for SamAttention/Sam2Attention#40667
Cyrilvallez merged 2 commits intohuggingface:mainfrom
yonigozlan:fix-training-sam

Conversation

@yonigozlan
Copy link
Copy Markdown
Member

What does this PR do?

Fixes #40255 (comment)

self.dropout_p shouldn't be used for SamAttention/Sam2Attention, only for RoPE attention (memory attention) in Sam2Video

@yonigozlan yonigozlan added the for patch Tag issues / labels that should be included in the next patch label Sep 3, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 3, 2025

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

run-slow: sam, sam2, sam2_video, sam_hq

Copy link
Copy Markdown
Contributor

@vasqu vasqu left a comment

Choose a reason for hiding this comment

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

Do the other models not have dropout anymore? See #40255 (comment) please, it was there before

@yonigozlan
Copy link
Copy Markdown
Member Author

It was there for VisionAttention (and still is ;) ), but not for the Attention class that is used in the mask decoder

Copy link
Copy Markdown
Contributor

@vasqu vasqu left a comment

Choose a reason for hiding this comment

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

Ah, now i see it - there are two attentions my bad

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

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.

@vasqu
Copy link
Copy Markdown
Contributor

vasqu commented Sep 3, 2025

Nit: Is there any tests for training? Might be good to add if you have time

Copy link
Copy Markdown
Member

@Cyrilvallez Cyrilvallez left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Cyrilvallez Cyrilvallez merged commit ad2da3e into huggingface:main Sep 4, 2025
19 checks passed
Cyrilvallez pushed a commit that referenced this pull request Sep 4, 2025
)

Fix dropout_p is not defined for SamAttention/Sam2Attention
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

for patch Tag issues / labels that should be included in the next patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SAM decoder training error

4 participants