-
Notifications
You must be signed in to change notification settings - Fork 557
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
Integration of OpenCLIP into FiftyOne Model Zoo #3925
Conversation
Hi @AdonaiVera thank you for the contribution, we appreciate it!! OpenCLIP has been near the top of our list of must-haves in the model zoo! It looks like you've included most of the source code from open_clip. We should not directly copy the source code but rather import the package. Could you try that, then push again here? It will make it much easier to review this PR also 😄 |
Hi @swheaton 👋 I've pushed the changes to the PR. Could you take a look when you have a moment? Also, I'm ready to integrate other methods into FiftyOne if that's helpful. Just let me know 🚀 |
Awesome thank you 🙇🏼
Do you mean other OpenCLIP methods or other models into model zoo? |
I'm open to contributing in any way that best serves the computer vision community. This could include adding more OpenCLIP methods or introducing new models to the model zoo. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #3925 +/- ##
===========================================
- Coverage 15.87% 15.86% -0.01%
===========================================
Files 731 731
Lines 81557 81665 +108
Branches 1093 1093
===========================================
+ Hits 12944 12959 +15
- Misses 68613 68706 +93
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
@AdonaiVera thank you for this contribution! Amazing! 🎉 💪
fiftyone/utils/open_clip.py
Outdated
self.context_length = self.parse_int(d, "context_length") | ||
self.text_prompt = self.parse_string(d, "text_prompt") | ||
|
||
self.model_name_clip = self.parse_string(d, "model_name_clip") |
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.
are model_name
and pretrained
required? If the OpenCLIP package defaults to something, you could update to this:
self.model_name = self.parse_string(d, "model_name", default=None)
self.pretrained = self.parse_string(d, "pretrained", default=None)
so that users could just do this:
model = foz.load_zoo_model("open-clip-torch")
without needing to know a particular pretrained model to load.
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.
Hi @brimoor,
Thanks a lot for your feedback! I'm working on the adjustments right now. Just to clarify, the 'pretrained' parameter is optional, but specifying the model name is necessary. To streamline the process, I'm setting the default model to 'ViT-B-32'. This should make it more user-friendly while ensuring essential parameters are in place.
…, and adjusted optional variable handling. These changes enhance code clarity and efficiency. Related to PR voxel51#3925
waiting for the merge, I need this |
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.
@AdonaiVera I'm going to go ahead and merge this so that we can include it in fiftyone==0.23.2
which is releasing today! I'll make the model_name
-> clip_model_name
change for you 😄
Thanks again for this great contribution!
…, and adjusted optional variable handling. These changes enhance code clarity and efficiency. Related to PR #3925
@AdonaiVera just a heads up: in testing this PR, I found that I needed to provide a So I updated the defaults in the version that will ship with
|
Actually, that pre-trained value is optional according to the documentation but it's okay to set OpenAI as the default value. Thank you for making the modification 💪 Looks amazing, thank you @brimoor 🚀 💯 |
Integration of YOLO-NAS into FiftyOne Model Zoo #3925
Hello FiftyOne Team and Community,
I'm thrilled to announce the integration of OpenCLIP into the FiftyOne Model Zoo. This addition enriches the Model Zoo with the capabilities of OpenCLIP, a powerful vision foundation model.
About the Integration:
Demo Notebook:
Available Models in OpenCLIP:
Installation and Usage:
Closing:
Cheers,
Adonai Vera