Feature/add pre postprocessing example #103

jafet-chaves · 2020-09-02T20:51:58Z

No description provided.

examples/r2i/mean_std_preprocessing.cc

examples/r2i/top_sort_postprocessing.cc

mmontero · 2020-09-08T16:10:08Z

examples/r2i/mean_std_preprocessing.cc

+                          r2i::ImageFormat::Id required_format) override {
+    r2i::RuntimeError error;
+    int width;
+    int height;


initialize variables

mmontero · 2020-09-08T16:11:48Z

examples/r2i/mean_std_preprocessing.cc

+    width = in_frame->GetWidth();
+    height = in_frame->GetHeight();
+
+    std::shared_ptr<float> processed_data = PreProcessImage((


this indentation looks wrong

mmontero · 2020-09-08T16:17:00Z

examples/r2i/mean_std_preprocessing.cc

+  }
+
+ private:
+  int required_width, required_height;


one variable per line

mmontero · 2020-09-08T16:34:15Z

examples/r2i/mean_std_preprocessing.cc

+
+    this->required_width = required_width;
+    this->required_height = required_height;
+    this->required_format = required_format;


why do you create these class variables? looks like you only use them on validate and you can pass them easily as parameters of the function

mmontero · 2020-09-08T16:39:37Z

examples/r2i/mean_std_preprocessing.cc

+                       required_height, 0, channels);
+
+    for (int i = 0; i < scaled_size; i += channels) {
+      /* RGB = (RGB - Mean)*StdDev */


RGB = (RGB - Mean) / StdDev ?

mmontero · 2020-09-08T16:42:18Z

examples/r2i/mean_std_preprocessing.cc

+
+class Example: public r2i::IPreprocessing {
+ public:
+  Example () {


why example? I already now this is an example we are on the examples directory, I suggest to use meaningful name

mmontero · 2020-09-09T15:35:08Z

examples/r2i/mean_std_preprocessing.cc

+    height = in_frame->GetHeight();
+
+    std::shared_ptr<float> processed_data = PreProcessImage(
+        reinterpret_cast<const unsigned char *>(in_frame->GetData()), width, height,


use a variable to hold the data as you did for width and height

mmontero · 2020-09-09T15:53:51Z

examples/r2i/mean_std_preprocessing.cc

+    this->scaled_ptr = std::shared_ptr<unsigned char>(new unsigned
+                       char[scaled_size],
+                       std::default_delete<const unsigned char[]>());
+    this->adjusted_ptr = std::shared_ptr<float>(new float[scaled_size],


why are you storing scaled and adjusted images ptr on the class? I think you can use local variables

mmontero · 2020-09-09T15:55:08Z

examples/r2i/onnxrt/inception.cc

-  stbir_resize_uint8(input, width, height, 0, scaled.get(), reqwidth,
-                     reqheight, 0, channels);
+std::shared_ptr<r2i::IFrame> LoadImage(const std::string &path, int reqwidth,
+                                       int reqheight,


variables name req_width and req_height for consistency

mmontero · 2020-09-09T16:11:06Z

examples/r2i/top_sort_postprocessing.cc

+
+class TopSortPostprocessing: public r2i::IPostprocessing {
+ public:
+  std::shared_ptr<r2i::IPrediction> Apply(std::shared_ptr<r2i::IPrediction>


rrcarlosrodriguez

Pre/Post implementations should be moved to r2i, they shouldn't be in examples
Pre/Post implementations should have tests

lumurillo · 2020-09-24T14:41:46Z

r2i/ipostprocessing.h

@@ -32,7 +33,8 @@ class IPostprocessing {
   * \param prediction returned from the model inference.
   * \return Error with a description message.
   */
-  virtual RuntimeError Apply(IPrediction &prediction) = 0;
+  virtual std::shared_ptr<r2i::IPrediction> Apply(


This interface will change with my latest PR.

examples/r2i/onnxrt/inception.cc

tests/unit/r2i/preprocessing/normalize.cc

michaelgruner · 2020-09-28T21:21:27Z

tests/unit/r2i/preprocessing/normalize.cc

+
+  error = preprocessing->Apply(in_frame, out_frame);
+  LONGS_EQUAL(r2i::RuntimeError::Code::NULL_PARAMETER, error.GetCode());
+}


There are other missing tests:

Invalid format?

Zero stddev?

Output height != input height

Output width != input width

Output format != output format

Input type not uchar

Output type not float

Included the following tests: *Zero standard deviation *Invalid input format id

lumurillo reviewed Sep 3, 2020

View reviewed changes

examples/r2i/mean_std_preprocessing.cc Outdated Show resolved Hide resolved

lumurillo reviewed Sep 3, 2020

View reviewed changes

examples/r2i/mean_std_preprocessing.cc Outdated Show resolved Hide resolved

lumurillo reviewed Sep 3, 2020

View reviewed changes

examples/r2i/top_sort_postprocessing.cc Outdated Show resolved Hide resolved

mmontero reviewed Sep 8, 2020

View reviewed changes

jafet-chaves force-pushed the feature/add-pre-postprocessing-example branch from 39fcdee to 41d4568 Compare September 9, 2020 15:47

mmontero reviewed Sep 9, 2020

View reviewed changes

rrcarlosrodriguez requested changes Sep 9, 2020

View reviewed changes

jafet-chaves requested review from michaelgruner, lumurillo, mmontero and rrcarlosrodriguez September 16, 2020 19:05

lumurillo reviewed Sep 24, 2020

View reviewed changes

michaelgruner requested changes Sep 28, 2020

View reviewed changes

jafet-chaves force-pushed the feature/add-pre-postprocessing-example branch from 7416e1e to 0d56830 Compare September 29, 2020 19:57

jafet-chaves mentioned this pull request Sep 30, 2020

Memory not accesible by the CPU is not supported in Preprocessing modules #107

Open

jafet-chaves force-pushed the feature/add-pre-postprocessing-example branch from f91f609 to 7de8d7a Compare October 1, 2020 01:46

jafet-chaves added 14 commits October 2, 2020 09:07

Expand IPre/IPostprocessing interfaces

fc7b02f

Add Pre/Postprocessing modules example

c39f477

Add missing header files in Pre/Post modules

a7361d7

Move mean and std deviation values as constant definitions

ab04940

Print top prediction inside post processing module

e77de21

Add logic in preprocess module to validate required size and format

1e07018

Change pre/post module name to be meaningful

6a214a5

Fix commentary description

b90a51f

Initialize function variables

7121aaf

Remove unneeded private member in preprocessing module

5bdb389

Add local variable to store pointer data

0405009

Rename function arguments for consistency

347ff4e

Remove unneeded class members in preprocessing module

de8f494

Add additional explanatory comments in post processing module

b82d094

jafet-chaves added 28 commits October 2, 2020 09:07

Remove magic numbers from Normalize module tests

0ef1858

Add preprocessing module for FaceNetV1 model

46a851c

Simplify IPreprocessing interface

8e4fee2

Extend preprocessing tests for model specific implementation

1020a77

Fix code documentation in IPreprocessing interface

ee38325

Remove unneeded allocation of output data in Normalize module

894c70c

Remove image scaling processing from Normalize module

4a3c9f3

Use new IPostprocessing interface in TopSort module

431c05c

Use a more significant variable name in stbi_load function

73cc1ed

Use correct error code for the type of errors in LoadImage function

26c3c46

Remove unneeded variables for include search paths in Meson

a863d28

Revert change of moving include search path for examples/r2i/common

4b22e56

Clean unused C++ standard library headers

3d8dc11

Use Google C++ standard header ordering

f5ea878

Create individual tests for each preprocessing module

4539d54

Use modern C++ syntax in vector-based for loops

71397d3

Factor code in Normalize tests to initialize dummy array

f0dceb6

Fix defines of mean and std to make it clear they are float values

25ee413

Fix Normalize tests to avoid using trivial mean and std values

efab88a

Add pointer guard in NormalizeFaceNetV1 module

a5733d3

Don't asumme size different from 0 in NormalizeFaceNetV1 module

f995c6b

Add checks to avoid using zero standard deviation in Normalize

8c9ee16

Avoid recompiling Normalize each time using Autotools

e84b688

Add missing internal dependency for preprocessing tests

3b0ebaf

Add logic to remove assumption of same image format in example

bafc354

Add logic to check for input format and dimensions in Normalize module

501b49b

Extend Normalize tests

989f342

Included the following tests: *Zero standard deviation *Invalid input format id

Make InferenceOutput a polymorphic class

0bf3c84

jafet-chaves force-pushed the feature/add-pre-postprocessing-example branch from 7de8d7a to 0bf3c84 Compare October 2, 2020 15:08

jafet-chaves merged commit 14b3b58 into dev-0.10 Oct 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/add pre postprocessing example #103

Feature/add pre postprocessing example #103

jafet-chaves commented Sep 2, 2020

mmontero Sep 8, 2020

mmontero Sep 8, 2020

mmontero Sep 8, 2020

mmontero Sep 8, 2020

mmontero Sep 8, 2020

mmontero Sep 8, 2020

mmontero Sep 9, 2020

mmontero Sep 9, 2020

mmontero Sep 9, 2020

mmontero Sep 9, 2020

rrcarlosrodriguez left a comment

lumurillo Sep 24, 2020

michaelgruner Sep 28, 2020

Feature/add pre postprocessing example #103

Feature/add pre postprocessing example #103

Conversation

jafet-chaves commented Sep 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rrcarlosrodriguez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment