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: onnx package conflict during setup #894

Merged
merged 26 commits into from
Feb 22, 2023
Merged

fix: onnx package conflict during setup #894

merged 26 commits into from
Feb 22, 2023

Conversation

ZiniuYu
Copy link
Member

@ZiniuYu ZiniuYu commented Feb 21, 2023

There is a package conflict with onnxruntime and onnxruntime-gpu in setup. If we install both packages, we are not able to run onnx using gpu. Installing only onnxruntime-gpu enables clip_server to run both on CPU and GPU.

This PR also corrects the instructions on running docker image in different runtimes.

This PR also replaces the broken images in test files

Notes: onnxruntime is fixed at 1.13.1 to pass the test_ranker
nvidia-tensorrt is fixed at 8.4.1.5 to pass trt related tests

@codecov
Copy link

codecov bot commented Feb 21, 2023

Codecov Report

Merging #894 (f029cb2) into main (8a576c5) will decrease coverage by 11.49%.
The diff coverage is 68.42%.

@@             Coverage Diff             @@
##             main     #894       +/-   ##
===========================================
- Coverage   83.08%   71.60%   -11.49%     
===========================================
  Files          22       23        +1     
  Lines        1531     1567       +36     
===========================================
- Hits         1272     1122      -150     
- Misses        259      445      +186     
Flag Coverage Δ
cas 71.60% <68.42%> (-11.49%) ⬇️

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

Impacted Files Coverage Δ
server/clip_server/model/tokenization.py 74.41% <41.66%> (-13.47%) ⬇️
server/clip_server/model/cnclip_model.py 77.27% <77.27%> (ø)
server/clip_server/model/clip_model.py 92.59% <100.00%> (+0.92%) ⬆️
server/clip_server/model/pretrained_models.py 98.43% <100.00%> (+0.02%) ⬆️
server/clip_server/executors/clip_tensorrt.py 0.00% <0.00%> (-94.60%) ⬇️
server/clip_server/model/clip_trt.py 0.00% <0.00%> (-85.72%) ⬇️
server/clip_server/model/trt_utils.py 0.00% <0.00%> (-83.52%) ⬇️
server/clip_server/model/clip_onnx.py 87.30% <0.00%> (+22.22%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

server/setup.py Outdated
'onnx',
'onnxmltools',
'onnxruntime',
Copy link
Member

Choose a reason for hiding this comment

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

As you mentioned, we can always install onnxruntime-gpu?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've tested on x86 machines with/without GPU, both work fine. The CI failed for unknown reasons

server/setup.py Outdated Show resolved Hide resolved
@ZiniuYu ZiniuYu linked an issue Feb 22, 2023 that may be closed by this pull request
@ZiniuYu ZiniuYu closed this Feb 22, 2023
@ZiniuYu ZiniuYu reopened this Feb 22, 2023
@github-actions
Copy link

📝 Docs are deployed on https://ft-fix-onnx-docker--jina-docs.netlify.app 🎉

@ZiniuYu ZiniuYu marked this pull request as ready for review February 22, 2023 09:59
@ZiniuYu ZiniuYu requested a review from a team February 22, 2023 10:00
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 merged commit d70f238 into main Feb 22, 2023
@numb3r3 numb3r3 deleted the fix-onnx-docker branch February 22, 2023 10:37
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.

Running Docker image with customized yaml doesnt work
2 participants