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

Added support of OpenVINO export #214

Merged
merged 12 commits into from
Jan 2, 2023
Merged

Conversation

AlexKoff88
Copy link
Contributor

PR content:

  • OpenVINO export (through ONNX)
  • Test
  • Notebook that demonstrates export and inference

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@tomaarsen
Copy link
Member

@AlexKoff88
#156 just got merged regarding ONNX exporting. It may allow this PR to be simplified due to the large overlap in changes. Furthermore, there was discussion regarding moving all code for exporting into a new exporters submodule: #156 (comment). This PR should follow suit, i.e. rebase/merge main and move the functionality and tests to the exporters folders, respectively.

  • Tom Aarsen

@AlexKoff88
Copy link
Contributor Author

@AlexKoff88 #156 just got merged regarding ONNX exporting. It may allow this PR to be simplified due to the large overlap in changes. Furthermore, there was discussion regarding moving all code for exporting into a new exporters submodule: #156 (comment). This PR should follow suit, i.e. rebase/merge main and move the functionality and tests to the exporters folders, respectively.

* Tom Aarsen

Hi Tom,

I've revised the code according to the recent changes and your suggestions. Please check again.

@tomaarsen
Copy link
Member

Hello @AlexKoff88,

I think we're moving in the right direction with these recent changes, but there are some issues still. openvino>=2022.3 cannot be installed. This is likely because 2022.3.0 only has pre-releases at the moment, as can be seen on its PyPI. Perhaps we can make this a bit more lenient with >=2022.2?` I'm unsure whether the latest pre-release version introduces must-have functionality for our purposes.

Beyond that, the CI is complaining about code quality. That can be resolved by running ./make style, which runs black and isort on most of the files in the project.

  • Tom Aarsen

@AlexKoff88
Copy link
Contributor Author

Hi,

I cleaned up the style.

OpenVINO 2022.3 is essential. I am ok to postpone the merge 1-2 weeks before it is available on PyPI.

@tomaarsen tomaarsen added exporting Regarding exporting SetFit models (e.g. ONNX) enhancement New feature or request labels Dec 20, 2022
@AlexKoff88
Copy link
Contributor Author

@tomaarsen, OpenVINO 2022.3 is available and OpenVINO export should work now. Please merge.

notebooks/openvino_inference.ipynb Show resolved Hide resolved
notebooks/openvino_inference.ipynb Show resolved Hide resolved
notebooks/openvino_inference.ipynb Show resolved Hide resolved
notebooks/openvino_inference.ipynb Show resolved Hide resolved
@tomaarsen
Copy link
Member

tomaarsen commented Dec 28, 2022

@AlexKoff88 I still have some tiny nits on the notebook that you've provided. Thanks a lot for it, by the way, it helps users out a lot.
As for the actual code, it looks good. I'll merge this when the notebook is updated!

@AlexKoff88
Copy link
Contributor Author

@AlexKoff88 I still have some tiny nits on the notebook that you've provided. Thanks a lot for it, by the way, it helps users out a lot. As for the actual code, it looks good. I'll merge this when the notebook is updated!

Thanks, @tomaarsen. I've updated the notebook according to your comments.

Copy link
Member

@tomaarsen tomaarsen left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for this PR and for following up on our requests. Happy new year!

@tomaarsen tomaarsen merged commit fe0e7f4 into huggingface:main Jan 2, 2023
@AlexKoff88
Copy link
Contributor Author

Looks great! Thanks for this PR and for following up on our requests. Happy new year!

Thanks for accepting the contribution! And Happy New Year!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exporting Regarding exporting SetFit models (e.g. ONNX)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants