mirror of
https://github.com/openai/CLIP.git
synced 2026-11-05 01:37:26 +00:00
[GH-ISSUE #152] Redundant ln_pre #131
Labels
No labels
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
openai/CLIP#131
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @carefree0910 on GitHub (Sep 12, 2021).
Original GitHub issue: https://github.com/openai/CLIP/issues/152
First of all, thanks for your great work!
And I found out that the
ln_preseems to be a redundant component:github.com/openai/CLIP@3b473b0e68/clip/model.py (L212)Because you've already applied
pre_normmechanism inResidualAttentionBlock.I'm just wondering whether this is intended or some kinds of 'typo', thanks in advance!
@jongwook commented on GitHub (Sep 24, 2021):
I think they're not completely redundant because the 'identity' connection in
ResidualAttentionBlockdoes not go through layernorms.@russelldc commented on GitHub (Sep 24, 2021):
I'm not sure this is relevant to this specific issue, but I found a significant difference while using CLIP to guide image generation.
I tried removing the use of that ln_pre in the VisionTransformer's forward(). It was no longer able to guide the generation to anything other than red/green stripes.
@carefree0910 commented on GitHub (Sep 24, 2021):
I agree with you that it has impact on the model, but I'm not sure whether this is 'intended', because other main stream implementations don't have this kind of
ln_prebefore the 'identity' connection 😉@carefree0910 commented on GitHub (Sep 24, 2021):
Not sure if I get you right, but when I say 'redundant' I mean maybe we can re-train the model without it.
If you are using a pre-trained model, the
ln_preis definitely useful because the pre-trained weights are trained based on it 😆@jongwook commented on GitHub (Oct 2, 2021):
We had it since when we initially iterated on transformer language model where it had ResNet features as prefix (LM RN50 in paper), and it got kinda stuck in the code for the contrastive version. I agree with you that it's not particularly necessary in the text encoder when training from scratch.