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

Naming clash in new CLIP models #240

Closed
StellaAthena opened this issue Nov 21, 2022 · 11 comments · Fixed by #251
Closed

Naming clash in new CLIP models #240

StellaAthena opened this issue Nov 21, 2022 · 11 comments · Fixed by #251

Comments

@StellaAthena
Copy link

I just cloned this repository on a Windows computer and saw the following:

PS C:\Users\585491\documents\research> git clone https://github.com/mlfoundations/open_clip.git
Cloning into 'open_clip'...
remote: Enumerating objects: 1637, done.
remote: Counting objects: 100% (74/74), done.
remote: Compressing objects: 100% (53/53), done.
remote: Total 1637 (delta 25), reused 49 (delta 17), pack-reused 1563
Receiving objects: 100% (1637/1637), 8.06 MiB | 10.91 MiB/s, done.
Resolving deltas: 100% (934/934), done.
warning: the following paths have collided (e.g. case-sensitive paths
on a case-insensitive filesystem) and only one from the same
colliding group is in the working tree:

  'src/open_clip/model_configs/ViT-G-14.json'
  'src/open_clip/model_configs/ViT-g-14.json'
  'tests/data/output/ViT-G-14_None_fp32_random_image.pt'
  'tests/data/output/ViT-g-14_None_fp32_random_image.pt'
  'tests/data/output/ViT-G-14_None_fp32_random_text.pt'
  'tests/data/output/ViT-g-14_None_fp32_random_text.pt'

It would be nice if the names could be adjusted to be compliant with case-insensitive file systems.

@rom1504
Copy link
Collaborator

rom1504 commented Nov 21, 2022 via email

@rwightman
Copy link
Collaborator

sigh, oh windows! it's possible to set per-dir case senstivity fsutil.exe file setCaseSensitiveInfo <path> disable filesystem supports it but it's left off since lots of apps don't , in wsl you can set it when you mount a path

g is in use out there, so wouldn't want to change that, we could change G to something else...

... one of the reasons I usually use snake case for names that are likely to be exposed to filesystem...

@usuyama
Copy link

usuyama commented Nov 23, 2022

WSL should be a good workaround, but feel like G vs g can be a bit confusing for humans too.

@lopho
Copy link
Contributor

lopho commented Nov 24, 2022

Just some proposals, if you decide to change the name, better sooner than later, before it's been too long in the wild.

vit-g-14-1280 (embed dim, vs ViT-g-14 with 1024, might confuse with other vit using this for the image input size, like ViT-L-14-336)
vit-ug-14 (upper g)
vit-gg-14 (a bit more giant than g)
vit-cg-14 (capital g)
vit-g2-14

@rwightman
Copy link
Collaborator

rwightman commented Nov 24, 2022

I was thinking 'gg' or just ViT-bigG-14.json we don't have to remove case itself, just ambiguity here when the names are treated as if there is no case difference as in windows

@rushin682
Copy link

Hello, I get the same warning for macOS (APFS file system):

Cloning into 'open_clip'...
remote: Enumerating objects: 1706, done.
remote: Counting objects: 100% (143/143), done.
remote: Compressing objects: 100% (110/110), done.
remote: Total 1706 (delta 63), reused 88 (delta 29), pack-reused 1563
Receiving objects: 100% (1706/1706), 8.09 MiB | 4.41 MiB/s, done.
Resolving deltas: 100% (972/972), done.
warning: the following paths have collided (e.g. case-sensitive paths
on a case-insensitive filesystem) and only one from the same
colliding group is in the working tree:

  'src/open_clip/model_configs/ViT-G-14.json'
  'src/open_clip/model_configs/ViT-g-14.json'
  'tests/data/output/ViT-G-14_None_fp32_random_image.pt'
  'tests/data/output/ViT-g-14_None_fp32_random_image.pt'
  'tests/data/output/ViT-G-14_None_fp32_random_text.pt'
  'tests/data/output/ViT-g-14_None_fp32_random_text.pt'

I am trying to do a model development, so it's not a big issue for me, but it might just be convenient to name them differently. Thanks.

@StellaAthena
Copy link
Author

I was thinking 'gg' or just ViT-bigG-14.json we don't have to remove case itself, just ambiguity here when the names are treated as if there is no case difference as in windows

I was going to recommend GG or BigG for similar reasons.

This was referenced Nov 24, 2022
@lopho
Copy link
Contributor

lopho commented Nov 24, 2022

Made two PRs, pick and choose ❤️
bigG #251
GG #252

@rom1504
Copy link
Collaborator

rom1504 commented Nov 24, 2022

I vote for bigG

@rwightman
Copy link
Collaborator

I'll second bigG

@rom1504
Copy link
Collaborator

rom1504 commented Nov 27, 2022

bigG merged

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 a pull request may close this issue.

6 participants