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.

Sign up for GitHub

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

Jump to bottom

Align openvino.compile_model and openvino.Core.compile_model functions #19778

Merged
merged 21 commits into from
Sep 21, 2023
Merged

Align openvino.compile_model and openvino.Core.compile_model functions #19778

merged 21 commits into from
Sep 21, 2023

Conversation

siddhant-0707
Copy link
Contributor

Details:

  • aligned openvino.compile_model and openvino.Core.compile_model functions
  • created test for openvino.compile_model

Tickets:

@siddhant-0707 siddhant-0707 requested a review from a team as a code owner September 12, 2023 12:48
@github-actions github-actions bot added the category: Python API OpenVINO Python bindings label Sep 12, 2023
@ilya-lavrenov ilya-lavrenov added this to the 2023.2 milestone Sep 12, 2023
@ilya-lavrenov ilya-lavrenov added the ExternalPR External contributor label Sep 12, 2023
@p-wysocki p-wysocki mentioned this pull request Sep 12, 2023
Closed
@ilya-lavrenov ilya-lavrenov linked an issue Sep 12, 2023 that may be closed by this pull request
Align openvino.compile_model and openvino.Core.compile_model functions #19616
Closed
@ilya-lavrenov
Copy link
Contributor

build_jenkins

p-wysocki
p-wysocki reviewed Sep 13, 2023
View reviewed changes
Copy link
Contributor

@p-wysocki p-wysocki left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! LGTM, I left one minor comment.

src/bindings/python/tests/test_runtime/test_core.py Outdated Show resolved Hide resolved
jiwaszki
jiwaszki requested changes Sep 13, 2023
View reviewed changes
Copy link
Contributor

@jiwaszki jiwaszki left a comment

Choose a reason for hiding this comment

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

One major thing to change. Tests are optional and I will really appreciate considering rewriting of them.

Overall good PR! 👍

src/bindings/python/src/openvino/runtime/ie_api.py Outdated Show resolved Hide resolved
src/bindings/python/tests/test_runtime/test_core.py Outdated Show resolved Hide resolved
@siddhant-0707
Copy link
Contributor Author

siddhant-0707 commented Sep 14, 2023 •
edited

Hey guys, I've tried to combine implementation of test_compact_api_xml_posix_path, test_compile_model_with_auto_plugin and test_compile_model_with_auto_plugin_no_default.

I've also combined test_compile_model and test_compile_model_without_device which are using openvino.Core.compile_model

For test_compile_model_with_auto_plugin and test_compile_model_with_auto_plugin_no_default, I've used the implementation @akuporos suggested

Please let me know if something needs fixed (probably does)

@p-wysocki
Copy link
Contributor

Hello @siddhant-0707, it seems there are some assertion errors in the Azure checks, could you please take a look?

Additionally, some Jenkins check require manual running by a maintainer, but you can also run them yourself by adding a GitHub comment with build_jenkins.

@siddhant-0707
Copy link
Contributor Author

build_jenkins

@siddhant-0707
reverted changes
63fa7df 
added `test_compile_model` and `test_compile_model_without_device` which test core.compile_model
@siddhant-0707
combined test_compile_model and test_compile_model_without_device…
9faab55 
… into `test_compile_model`
@siddhant-0707
Copy link
Contributor Author

Hey @akuporos I've combined test_compile_model and test_compile_model_without_device which are tests for core.compile_model, and given them the name test_compile_model itself.

@siddhant-0707 @akuporos
update test_core.py
44fe6e2 
Co-Authored-By: Anastasia Kuporosova <anastasia.kuporosova@intel.com>
@akuporos
Copy link
Contributor

build_jenkins

@akuporos
Copy link
Contributor

@siddhant-0707 please, fix pylint checks

siddhant-0707 and others added 2 commits September 20, 2023 21:01
@siddhant-0707 @akuporos
fix
2906a5a 
Co-Authored-By: Anastasia Kuporosova <anastasia.kuporosova@intel.com>
@akuporos
Merge branch 'master' into align-compile-model
97d982d
@akuporos
Copy link
Contributor

build_jenkins

@akuporos
Copy link
Contributor

build_jenkins

jiwaszki
jiwaszki approved these changes Sep 21, 2023
View reviewed changes
Copy link
Contributor

@jiwaszki jiwaszki left a comment

Choose a reason for hiding this comment

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

Great work! Thanks for this contribution 🥇

@jiwaszki jiwaszki enabled auto-merge (squash) September 21, 2023 08:50
@jiwaszki jiwaszki merged commit d6d4e3d into openvinotoolkit:master Sep 21, 2023
59 checks passed
@siddhant-0707
Copy link
Contributor Author

Thank you so much for the help @akuporos and @jiwaszki !!

@siddhant-0707 siddhant-0707 deleted the align-compile-model branch October 25, 2023 08:19
alvoron pushed a commit to alvoron/openvino that referenced this pull request Nov 6, 2023
@siddhant-0707 @akuporos @alvoron
Align openvino.compile_model and openvino.Core.compile_model functions (
aa8f3ac 
openvinotoolkit#19778)

Co-Authored-By: Anastasia Kuporosova <anastasia.kuporosova@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akuporos akuporos akuporos approved these changes

@p-wysocki p-wysocki p-wysocki approved these changes

@jiwaszki jiwaszki jiwaszki approved these changes

Assignees

@p-wysocki p-wysocki

Labels
category: Python API OpenVINO Python bindings ExternalPR External contributor
Projects
None yet
Milestone
2023.2
Development

Successfully merging this pull request may close these issues.

Align openvino.compile_model and openvino.Core.compile_model functions
5 participants
@siddhant-0707 @ilya-lavrenov @p-wysocki @akuporos @jiwaszki