Skip to content
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

Performs normalization of the text embedding twice #90

Closed
Adamdad opened this issue Apr 3, 2023 · 2 comments
Closed

Performs normalization of the text embedding twice #90

Adamdad opened this issue Apr 3, 2023 · 2 comments

Comments

@Adamdad
Copy link

Adamdad commented Apr 3, 2023

The zeroshot_classification.py script includes code (https://github.com/LAION-AI/CLIP_benchmark/blob/main/clip_benchmark/metrics/zeroshot_classification.py#L50) that performs normalization of the text embedding twice. Specifically, the F.normalize function from PyTorch is called to normalize the text embedding along the last dimension, and the resulting tensor is then averaged along the first dimension to obtain a single embedding vector. However, the code then repeats the normalization step on this single embedding vector using class_embedding.norm(). This second normalization appears to be redundant and can be safely removed.

class_embeddings = model.encode_text(texts)
class_embedding = F.normalize(class_embeddings, dim=-1).mean(dim=0)
class_embedding /= class_embedding.norm() # Repeat Normalization
@djghosh13
Copy link
Contributor

I believe the second norm is necessary; note that class_embedding is the mean of multiple class_embeddings and isn't guaranteed to be normalized.

@mehdidc
Copy link
Collaborator

mehdidc commented Jul 7, 2023

Indeed, after averaging normalization is not guaranteed anymore, so this is still needed.

@mehdidc mehdidc closed this as completed Jul 7, 2023
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

No branches or pull requests

3 participants