implemented getOutputElementType #25657
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
hey @Pey-crypto there is a conflict to solve, will you have a time to fix it, please? |
|
Will do it soon👍 |
|
awesome, thank you in advance! |
almilosz
requested changes
Jul 25, 2024
Comment on lines
112
to
117
| /** | ||
| * @brief Helper function to access model output elements types. | ||
| * @return Napi::String representing the element type of the requested output. | ||
| */ | ||
|
|
||
| Napi::Value get_output_element_type(const Napi::CallbackInfo& info); |
There was a problem hiding this comment.
Suggested change
| /** | |
| * @brief Helper function to access model output elements types. | |
| * @return Napi::String representing the element type of the requested output. | |
| */ | |
| Napi::Value get_output_element_type(const Napi::CallbackInfo& info); | |
| /** | |
| * @brief Helper function to access model output elements types. | |
| * @return Napi::String representing the element type of the requested output. | |
| */ | |
| Napi::Value get_output_element_type(const Napi::CallbackInfo& info); |
src/bindings/js/node/lib/addon.ts
Outdated
Comment on lines
238
to
241
| * It gets the input of the model. | ||
| * If a model has more than one input, this method throws an exception. | ||
| */ | ||
| getOutputElementType(index: number): string; | ||
| /** | ||
| * It gets the element type of a specific output of the model. | ||
| */ | ||
| input(): Output; |
There was a problem hiding this comment.
Suggested change
| * It gets the input of the model. | |
| * If a model has more than one input, this method throws an exception. | |
| */ | |
| getOutputElementType(index: number): string; | |
| /** | |
| * It gets the element type of a specific output of the model. | |
| */ | |
| input(): Output; | |
| * It gets the element type of a specific output of the model. | |
| * @param index The index of the output. | |
| */ | |
| getOutputElementType(index: number): string; | |
| /** | |
| * It gets the input of the model. | |
| * If a model has more than one input, this method throws an exception. | |
| */ | |
| input(): Output; |
Typedoc comments have to be above the method you are adding
| reportError(info.Env(), e.what()); | ||
| return info.Env().Undefined(); | ||
| } | ||
| } |
| /^Error:/, | ||
| 'Should throw for out-of-range index' | ||
| ); | ||
| }); | ||
| }); |
| #include "node/include/addon.hpp" | ||
| #include "node/include/errors.hpp" | ||
| #include "node/include/helper.hpp" | ||
| #include "node/include/model_wrap.hpp" |
There was a problem hiding this comment.
Please revert this change and move #include "node/include/model_wrap.hpp" to the top.
Probably you are using a different formatting tool than is specified here
| auto output = _model->output(idx); | ||
| return cpp_to_js<ov::element::Type_t, Napi::String>(info, output.get_element_type()); | ||
| } else { | ||
| OPENVINO_THROW("getOutputElementType", ov::js::get_parameters_error_msg(info, allowed_signatures)); |
There was a problem hiding this comment.
Suggested change
| OPENVINO_THROW("getOutputElementType", ov::js::get_parameters_error_msg(info, allowed_signatures)); | |
| OPENVINO_THROW("'getOutputElementType'", ov::js::get_parameters_error_msg(info, allowed_signatures)); |
| it('should accept a single integer argument', () => { | ||
| assert.throws(() => { | ||
| model.getOutputElementType(); | ||
| }, /^Error: getOutputElementType method called with incorrect parameters\./, |
There was a problem hiding this comment.
You forgot to update error messages ;)
|
|
||
| assert.throws(() => { | ||
| model.getOutputElementType('unexpected argument'); | ||
| }, /^Error: getOutputElementType method called with incorrect parameters\./, |
|
|
||
| assert.throws(() => { | ||
| model.getOutputElementType(0, 1); | ||
| }, /^Error: getOutputElementType method called with incorrect parameters\./, |
|
|
||
| assert.throws(() => { | ||
| model.getOutputElementType(3.14); | ||
| }, /^Error: getOutputElementType method called with incorrect parameters\./, |
Pey-crypto
force-pushed
the
outputDataType
branch
from
118807c to
3056b53
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented Method on c++ side.
Updated typescript definitions.
Created unit tests.
For Issue #25406