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

Readability of CLIP notebook #28

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

josh-freeman
Copy link
Contributor

  • deleted modification of clip.clip.MODELS as it is no longer needed (all models are now there by default, see last commit)

  • there was a repetition of 17 lines of code twice in interpret. I replaced it with a function. I also added to this function an assertion that checks if the model is coming from Hila Chefer's version of CLIP or an equivalent (https://github.com/hila-chefer/Transformer-MM-Explainability/tree/main/CLIP).

  • added a function mask_from_relevance for users who would want to display the attention mask in another way than show_cam_on_image.

- deleted modification of clip.clip.MODELS as it is no longer needed (all models are now there by default, see last commit)
- there was a repetition of 17 lines of code twice in `interpret`. I replaced it with a function. I also added to this function an assertion that checks if the model is coming from Hila Chefer's version of CLIP or an equivalent (https://github.com/hila-chefer/Transformer-MM-Explainability/tree/main/CLIP). 

- added a function `mask_from_relevance` for users who would want to display the attention mask in another way than `show_cam_on_image`.
@josh-freeman
Copy link
Contributor Author

Oh I forgot to mention:

I also added a bit of documentation to the interpret function

@hila-chefer
Copy link
Owner

Hi @josh-freeman, thanks for your contribution to this repo! it'll take me some time to review and approve your PR since it contains a significant amount of changes, will get to it ASAP

@josh-freeman
Copy link
Contributor Author

No worries, pretty sure it's mostly a bug coming from something like conversion of CRLF to LF tokens or something; I'm surprised it says I changed that much.

@guanhdrmq
Copy link

Dear all,

I have one problem for ViLT. So I try to reproduce the VisualBERT for implementing in VILT. Could you point where is the save_visual_results function definition? I use ViLT for multimodal transformer but cannot use num_tokens = image_attn_blocks[0].attn_probs.shape[-1] to set the number of tokens. For example, VILT for VQA task and image is 384*384 size. The number of vision and text mixed token is 185 including cls token, so the vision token is 144 and the text token is 40 (max length).

Thanks very much

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

Successfully merging this pull request may close these issues.

3 participants