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

fix: add models and md5 #783

Merged
merged 6 commits into from
Jul 26, 2022
Merged

fix: add models and md5 #783

merged 6 commits into from
Jul 26, 2022

Conversation

ZiniuYu
Copy link
Member

@ZiniuYu ZiniuYu commented Jul 25, 2022

This pr adds open-clip model filenames and their corresponding md5
We now host a copy of open-clip models and convert them to onnx runtime

@codecov
Copy link

codecov bot commented Jul 25, 2022

Codecov Report

Merging #783 (3d24db6) into main (7c8285b) will decrease coverage by 3.33%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #783      +/-   ##
==========================================
- Coverage   86.64%   83.30%   -3.34%     
==========================================
  Files          21       21              
  Lines        1108     1108              
==========================================
- Hits          960      923      -37     
- Misses        148      185      +37     
Flag Coverage Δ
cas 83.30% <100.00%> (-3.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
server/clip_server/executors/clip_torch.py 87.93% <ø> (ø)
server/clip_server/model/clip_onnx.py 88.23% <ø> (ø)
server/clip_server/model/pretrained_models.py 84.12% <ø> (-1.59%) ⬇️
server/clip_server/model/openclip_model.py 78.57% <100.00%> (-10.72%) ⬇️
server/clip_server/model/trt_utils.py 56.04% <0.00%> (-27.48%) ⬇️
server/clip_server/model/clip_trt.py 71.05% <0.00%> (-21.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c8285b...3d24db6. Read the comment docs.

@@ -24,7 +24,7 @@ def __init__(self, name: str, device: str = 'cpu', jit: bool = False, **kwargs):
if model_url:
model_path = download_model(model_url, md5sum=md5sum)
self._model = load_openai_model(model_path, device=device, jit=jit)
self._model_name = name
self._model_name = name.split('::')[0]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@numb3r3 Check this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@numb3r3 Check this

should be split[0], I had fixed it in my pr

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem caused here can be addressed by another PR. So let's revert this change in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need this to pass ci

@@ -24,7 +24,7 @@ def __init__(self, name: str, device: str = 'cpu', jit: bool = False, **kwargs):
if model_url:
model_path = download_model(model_url, md5sum=md5sum)
self._model = load_openai_model(model_path, device=device, jit=jit)
self._model_name = name
self._model_name = name.split('::')[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem caused here can be addressed by another PR. So let's revert this change in this PR.

Copy link
Member

@numb3r3 numb3r3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@numb3r3 numb3r3 marked this pull request as ready for review July 26, 2022 02:19
@numb3r3 numb3r3 merged commit 5877207 into main Jul 26, 2022
@numb3r3 numb3r3 deleted the update_models branch July 26, 2022 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants