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

use onnxruntime-gpu for large-model-test #24

Open
wants to merge 6 commits into
base: mainline
Choose a base branch
from

Conversation

wanliAlex
Copy link
Contributor

use onnxruntime-gpu for large-model-test

@wanliAlex wanliAlex requested a review from vicilliar April 24, 2023 06:08
@@ -96,5 +97,7 @@ setenv =
PATH = {env:PATH}{:}{toxinidir}{/}scripts
commands =
pip install --upgrade -r {toxinidir}{/}temp{/}marqo{/}requirements.txt
pip uninstall -y onnxruntime
pip install onnxruntime-gpu==1.13.1
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think tox envs were meant to use deps like this, why not instead do this:

  • don't use pip install
  • install deps with
deps =
  {[testenv]deps}
  -r{toxinidir}{/}temp{/}marqo{/}requirements.txt
  • define onnxruntime-gpu==1.13.1 in requirements.txt

But if it's not possible to change this in the requirements file itself, your solution could be a good workaround :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the requirements.txt is using onnxruntime in general. We only need onnxruntime-gpu for the large model test. any good idea?

Copy link
Contributor

@vicilliar vicilliar Apr 26, 2023

Choose a reason for hiding this comment

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

I see. note that the solution you put here will affect both largemodel_unit_test_CI.yml and unit_test_CI.yml. Have you tried running regular unit tests with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i missed this point. we should use the onnxruntime in unittest as the instance does not have GPU. do you reckon creating a new test env will solve the problem?

@wanliAlex
Copy link
Contributor Author

wanliAlex commented Apr 26, 2023

image
correctly uninstall onnxruntime and install onnxruntime-gpu

image

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.

2 participants