-
Notifications
You must be signed in to change notification settings - Fork 529
[MAEB] Add proper wav2clip text encoder #3849
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
[MAEB] Add proper wav2clip text encoder #3849
Conversation
| self.clip = CLIPModel.from_pretrained(model_name, revision=revision).to(device) | ||
| # text side (CLIP) - we use the standard OpenAI CLIP model as mentioned in paper | ||
| clip_model_name = "openai/clip-vit-base-patch32" | ||
| self.clip = CLIPModel.from_pretrained(clip_model_name).to(device) |
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.
add revision as well
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.
You should be able to load it directly onto the device using the device_map
| # Pad with zeros | ||
| pad_length = max_length - wav.shape[-1] | ||
| padded_wav = torch.nn.functional.pad(wav, (0, pad_length)) | ||
| padded_wav = np.pad(wav, (0, pad_length), mode="constant") |
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.
torch is fine - no need to change
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.
Changed back. I think some dataset gave an error when using torch but can't remember which or if it was because of something else. For now, changed back to torch. They are equivalent in 1D audio arrays.
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.
Can you remove link to submodule?
|
I think this the same as #3781 |
|
Yup closing the other one. |
|
@Samoed Anything else needed here? |
If you add a model or a dataset, please add the corresponding checklist: