CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
G-API: Documentation for Params (IE and ONNX). #20112
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
Conversation
BTW, In documentation PRs it makes sense to update PR's description with link on generated affected documentation pages (after "Docs" builder is completed). Link should point somewhere here: https://pullrequest.opencv.org/buildbot/export/pr/20112/docs/ This is extremely useful for reviewers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start! Need to make it a little bit more comprehensive
f3a401a
to
d23d02b
Compare
@@ -117,75 +140,180 @@ template<typename Net> class Params { | |||
, 1u} { | |||
}; | |||
|
|||
Params<Net>& cfgInputLayers(const typename PortCfg<Net>::In &ll) { | |||
/** @brief Specifies sequence of CNN input layers names for inference. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't hardcode to CNN
networks, it works with any networks right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed. network instead CNN.
Params& pluginConfig(const IEConfig& cfg) { | ||
desc.config = cfg; | ||
/** @overload | ||
Function with an rvalue parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an
-> a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
std::string device_id; //!< Device specifier. | ||
|
||
// NB: Here order follows the `Net` API | ||
std::vector<std::string> input_names; //!< Names of input CNN layers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Names of input network layers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would stop using CNN
for all entities below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please align the ONNX documentation with the feedback on the IE stuctures (hide paramdesc, etc)
return *this; | ||
} | ||
|
||
/** @overload | ||
Function for reshape by image size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function doesn't reshape any images
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function is used to set names of layers that will be used for network reshape. | ||
Dimensions will be constructed automatically by current network input and height and | ||
width of image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please align with the rest of the reshape documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Function with a rvalue parameters for reshape by image size. | ||
|
||
@param layer_names rvalue set of the selected layers will be reshaped automatically | ||
its input image size. | ||
@return the reference on modified object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to document the rvalue specifics, simply mention the overload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@mpashchenkov is there any updates atop of the last (6d ago) review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TolyaTalamanov please review my comments carefully.
0da717b
to
74d5454
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! Please go ahead with the merge (cc: @alalek )
G-API: Documentation for Params (IE and ONNX). * Applying comments * Removed type of model from PramsDesc * Added message for onnx ParamDesc * Whitespaces * Review * Fix comments to review * Fix comments Co-authored-by: Anatoliy Talamanov <anatoliy.talamanov@intel.com>
Documentation for cfg_functions of Params.
Docs: https://pullrequest.opencv.org/buildbot/export/pr/20112/docs/
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.
Magic commands: