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

Add Test for Optimum Intel #2674

Merged
merged 16 commits into from
Jun 3, 2024
Merged

Add Test for Optimum Intel #2674

merged 16 commits into from
Jun 3, 2024

Conversation

NoushNabi
Copy link
Contributor

@NoushNabi NoushNabi commented May 23, 2024

Added test for Optimum Intel.

@yifanmai
Copy link
Collaborator

Hi @NoushNabi, could you move this test into its own new job under jobs? It is not a unit test and should not be grouped with that.

You can do this by duplicating and modifying the Run HELM with minimal dependencies only test - in particular, you will have to run python3 -m pip install dist/crfm_helm-*.whl[openvino] instead of python3 -m pip install dist/crfm_helm-*.whl.

@NoushNabi
Copy link
Contributor Author

Hi @yifanmai
python3 -m pip install dist/crfm_helm-*.whl[openvino] was giving error since building was only producing crfm_helm-0.5.1-py3-none-any.whl. I added python3 -m pip install optimum[openvino] and it works on my machine but on Github Actions exits with error as Error: The operation was canceled. Can you please advise?

@yifanmai
Copy link
Collaborator

I think the following command should work i.e. resolve the wildcard first before passing the file name to pip:

python3 -m pip install "$(ls dist/crfm_helm-*.whl)[openvino]"

@NoushNabi
Copy link
Contributor Author

NoushNabi commented Jun 3, 2024

@yifanmai
Thanks for your reply. I appreciate it!
Finally all tests for Optimum Intel pass successfully.

@yifanmai
Copy link
Collaborator

yifanmai commented Jun 3, 2024

Lint is failing with:

src/helm/clients/huggingface_client.py:77:17: F401 'pathlib.Path' imported but unused

Could you please fix this?

@NoushNabi
Copy link
Contributor Author

@yifanmai
Sorry that I missed it. Done!

Copy link
Collaborator

@yifanmai yifanmai left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@yifanmai yifanmai merged commit b2378e4 into stanford-crfm:main Jun 3, 2024
9 checks passed
xuwangyin pushed a commit to xuwangyinx/helm that referenced this pull request Jun 23, 2024
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