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

Textual .forward() override bug. #12

Open
Great-Frosty opened this issue Apr 25, 2023 · 3 comments
Open

Textual .forward() override bug. #12

Great-Frosty opened this issue Apr 25, 2023 · 3 comments

Comments

@Great-Frosty
Copy link

Hi! First of all, thanks a lot for a great example project!
However, I'm pretty sure I have found a bug. onnx_model.encode_text() produces results that are not consistent with regular model output.
There is a statement in .utils:
take features from the eot embedding (eot_token is the highest number in each sequence).

But ruclip uses eos_id = 3, which clearly is not the highest token in the sequence.
This change was clearly made after you released you repo, but there was no version bump.
So, I tried tracing the model with hardcode eos id and original .where line from ruclip's encode_text:
x = x[torch.arange(x.shape[0]), torch.where(text == 3)[1]] @ self.text_projection
and it worked. I will submit a pull request with a fix, if I have free time later, but for now just wanted you to know, that running the default version of your notebook produces incorrect text_encoding results

@yiluzhou1
Copy link

@Great-Frosty I also found this bug when trying the openclip model (https://github.com/mlfoundations/open_clip). The output from .encode_text() is different from openclip model.

@GodTamIt
Copy link

@Great-Frosty were you ever able to take a look at this?

I'm unsure if this affects only ruclip or other clips as well.

@Great-Frosty
Copy link
Author

Great-Frosty commented Jan 18, 2024

@GodTamIt Me neither. It got fixed in ruclip though
ai-forever/ru-clip#19

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