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

Support core.query_model() #25016

Closed
wants to merge 4 commits into from

Conversation

hegdeadithyak
Copy link
Contributor

@hegdeadithyak hegdeadithyak commented Jun 13, 2024

Details:

Added Query Model to C++ Core Class and Updated TS addons

Solves : #24373

@hegdeadithyak hegdeadithyak requested a review from a team as a code owner June 13, 2024 18:55
@github-actions github-actions bot added the category: JS API OpenVino JS API Bindings label Jun 13, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label Jun 13, 2024
@hegdeadithyak
Copy link
Contributor Author

@vishniakov-nikolai I need some help with writing Tests for this.

@hegdeadithyak hegdeadithyak changed the title [OV JS] Support core.query_model() Support core.query_model() Jun 15, 2024

Napi::Value CoreWrap::query_model(const Napi::CallbackInfo& info) {
try {
if (info.Length() < 2 || !info[0].IsObject() || !info[1].IsString()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should check if info[0] contain model object like this:
const auto& model_prototype = info.Env().GetInstanceData<AddonData>()->model; if (model_prototype && model.InstanceOf(model_prototype.Value().As<Napi::Function>())) { }
The exact form of this condition I leave to you ;)

@@ -68,7 +68,17 @@ class CoreWrap : public Napi::ObjectWrap<CoreWrap> {
Napi::Value set_property(const Napi::CallbackInfo& info);
Napi::Value get_property(const Napi::CallbackInfo& info);

/** @brief Query device if it supports the specified model with specified properties.
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this description to addon.ts
You will have to rewrite it according to typedoc
You can look at how other methods in addon.ts are described

@almilosz
Copy link
Contributor

@vishniakov-nikolai I need some help with writing Tests for this.

The test should be put in this file: src/bindings/js/node/tests/core.test.js
You can use model from here -> src/bindings/js/node/tests/test_models
To test the functionality you can check if each operation in the output of the method is supported by the selected device
Something like this:

query = core.query_model(model=model, device_name=device)
assert device in next(iter(set(query.values()))), "Wrong device for some layers"

@mlukasze mlukasze linked an issue Jun 18, 2024 that may be closed by this pull request
Copy link
Contributor

This PR will be closed in a week because of 2 weeks of no activity.

@github-actions github-actions bot added the Stale label Jul 21, 2024
@vishniakov-nikolai
Copy link
Contributor

Hi @hegdeadithyak! To prevent close of this PR, please add fresh commit to it or let us know if you need more time. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: JS API OpenVino JS API Bindings ExternalPR External contributor Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Good First Issue]: [OV JS] Support core.query_model()
4 participants