G-API: API for configuring model output precision for IE backend by TolyaTalamanov · Pull Request #22583 · opencv/opencv

TolyaTalamanov · 2022-09-28T14:03:13Z

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

Overview

This cfgOutputPrecision handle is partially introduced/discussed there: #22522

TolyaTalamanov · 2022-09-28T14:04:22Z

@smirnov-alexey Could you have a look, please?

modules/gapi/src/backends/ie/giebackend.cpp

TolyaTalamanov · 2022-09-28T14:07:17Z

modules/gapi/include/opencv2/gapi/infer/ie.hpp

+
+    The function is used to set an output precision for model.
+
+    @param precision Precision in OpenCV format.


Need to explain what is the opencv format.
@alalek Could you explain how to point to opencv types: https://docs.opencv.org/4.x/d1/d1b/group__core__hal__interface.html#ga32b18d904ee2b1731a9416a8eef67d06 in doxygen.

(CV_8U, CV_32F, ...) ?

TolyaTalamanov · 2022-09-28T14:08:49Z

modules/gapi/src/backends/ie/giebackend.cpp

+                                     const ParamDesc::precision_variant_t &output_precision) {
+    switch (output_precision.index()) {
+        case ParamDesc::precision_variant_t::index_of<ParamDesc::precision_t>(): {
+            auto precision    = toIE(cv::util::get<ParamDesc::precision_t>(output_precision));


extra space

modules/gapi/src/backends/ie/giebackend.cpp

smirnov-alexey

Overall looks great, thanks!

TolyaTalamanov · 2022-09-29T15:06:50Z

@dmatveev Could you have a look, please?

dmatveev

Fine with changes but please keep comments in mind

dmatveev · 2022-10-03T03:07:08Z

modules/gapi/include/opencv2/gapi/infer/ie.hpp

+    // NB: cv::util::monostate is default value that means precision wasn't specified.
+    using precision_variant_t = cv::util::variant<cv::util::monostate,
+                                                  precision_t,
+                                                  precision_map_t>;


Can you please document the cases in this variant?

My understanding (I haven't read the rest of the patch yet) is that precision_t case is for single output layer, right? And map is for multi-output case?

No, user can do:

params.cfgOutputPrecision(`CV_16F`) // will be applied for all layers.

or specify certain layers:

params.cfgOutputPrecision({{"layer0", CV_16F}}); // only layer0 has changed precision

// will be applied for all layers

Well I failed to guess that. :)

dmatveev · 2022-10-03T03:07:39Z

modules/gapi/include/opencv2/gapi/infer/ie.hpp

              , {}
              , {}
              , {}
+              , {}


It must be reorganized, I'm afraid of touching it, since it'll cause lot's of changes.

dmatveev · 2022-10-03T03:12:09Z

modules/gapi/include/opencv2/gapi/infer/ie.hpp

+
+    The function is used to set an output precision for model.
+
+    @param precision Precision in OpenCV format.


(CV_8U, CV_32F, ...) ?

dmatveev · 2022-10-03T03:12:37Z

modules/gapi/include/opencv2/gapi/infer/ie.hpp

+    @param precision Precision in OpenCV format.
+    @return reference to this parameter structure.
+    */
+    Params<Net>& cfgOutputPrecision(detail::ParamDesc::precision_t precision) {


ah this mixture of CamelCase and snake_case_t :)

dmatveev · 2022-10-03T03:13:34Z

modules/gapi/include/opencv2/gapi/infer/ie.hpp

        : desc{ model, weights, device, {}, {}, {}, 0u, 0u,
                detail::ParamDesc::Kind::Load, true, {}, {}, {}, 1u,
-                {}, {}, {}, {}},
+                {}, {}, {}, {}, {}},


dmatveev · 2022-10-03T03:14:44Z

modules/gapi/samples/pipeline_modeling_tool/pipeline_builder.hpp

    std::vector<std::string> input_layers;
    std::vector<std::string> output_layers;
    std::map<std::string, std::string> config;
+    cv::util::optional<int> out_precision;


So here it is int, not a map of ints?

Config has parameter output_precision which is applied for all output layers. There is no need to have per layer precision for tool (so far...)

dmatveev · 2022-10-03T03:15:34Z

modules/gapi/samples/pipeline_modeling_tool/pipeline_builder.hpp

+    if (infer_params.out_precision) {
+        pp->cfgOutputPrecision(infer_params.out_precision.value());
+    }


No multi-output cases supported? What if the user writes such a config? Would it be rejected gracefully?

Multi-output case is supported. User can only define precision for all output layers. Answered above

dmatveev · 2022-10-03T03:16:59Z

modules/gapi/src/backends/ie/giebackend.cpp

+    const auto ie_type = toCV(desc.getPrecision());
+    if (ie_type != mat.type()) {
+        std::stringstream ss;
+        ss << "Failed while copying blob from IE to OCV: "


Failed to copy

Also, I doubt if multi-line exception text is useful. At least a single-liner is easier to grep

dmatveev · 2022-10-03T03:18:08Z

modules/gapi/src/backends/ie/giebackend.cpp

+static void configureOutputPrecision(const IE::OutputsDataMap             &outputs_info,
+                                     const ParamDesc::precision_variant_t &output_precision) {
+    switch (output_precision.index()) {
+        case ParamDesc::precision_variant_t::index_of<ParamDesc::precision_t>(): {


y not visit? we have one

TolyaTalamanov · 2022-10-03T08:09:31Z

modules/gapi/test/infer/gapi_infer_ie_test.cpp

    }
 }

+TEST(TestAgeGenderIE, ChangeOutputPrecision)


It must be refactoring asap. Adding new test brings lot's of copy-paste stuff.

TolyaTalamanov · 2022-10-03T09:09:40Z

@dmatveev @asmorkalov Do you mind merging it?

TolyaTalamanov added the category: g-api / gapi label Sep 28, 2022

TolyaTalamanov added this to the 4.7.0 milestone Sep 28, 2022

smirnov-alexey reviewed Sep 28, 2022

View reviewed changes

modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved

TolyaTalamanov commented Sep 28, 2022

View reviewed changes

smirnov-alexey reviewed Sep 28, 2022

View reviewed changes

modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved

smirnov-alexey approved these changes Sep 28, 2022

View reviewed changes

TolyaTalamanov force-pushed the at/add-cfg-output-precision-for-ie-backend branch from 0ea293f to 0166437 Compare October 2, 2022 20:54

dmatveev approved these changes Oct 3, 2022

View reviewed changes

TolyaTalamanov added 2 commits October 3, 2022 08:04

Add cfgOutputPrecision

b0b77b3

Fix comments to review

a6fbd82

TolyaTalamanov force-pushed the at/add-cfg-output-precision-for-ie-backend branch from 0166437 to a6fbd82 Compare October 3, 2022 08:05

TolyaTalamanov commented Oct 3, 2022

View reviewed changes

asmorkalov merged commit 7208f63 into opencv:4.x Oct 3, 2022

alalek mentioned this pull request Jan 8, 2023

(5.x) Merge 4.x #23113

Merged


		The function is used to set an output precision for model.

		@param precision Precision in OpenCV format.

Uh oh!

Conversation

TolyaTalamanov commented Sep 28, 2022

Pull Request Readiness Checklist

Overview

Uh oh!

TolyaTalamanov commented Sep 28, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

smirnov-alexey left a comment

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov commented Sep 29, 2022

Uh oh!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov Oct 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov Oct 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov commented Oct 3, 2022

Uh oh!

Reviewers

Assignees

Labels

TolyaTalamanov Oct 3, 2022 •

edited

Loading

TolyaTalamanov Oct 3, 2022 •

edited

Loading