C-API quantization core 2 #16396

wojtuss · 2019-03-22T11:25:19Z

This is a cleaned version of PR #15987.
It contains the core of the second version of INT8 quantization, C-API based only.

test=develop

Co-authored-by: Sylwester Fraczek sylwester.fraczek@intel.com

wojtuss · 2019-03-25T08:58:39Z

@luotao1 , I have restarted the failed build, is it possible to spur the CI to start the build faster?

luotao1 · 2019-03-25T09:26:22Z

You can restart the build, but this PR doesn't solve #15987 (comment)

luotao1 · 2019-03-25T09:29:15Z

Besides, to speed up review and merge. You can put the change of location modification into another PR, such as

wojtuss · 2019-03-25T10:08:54Z

@luotao1 ,

Besides, to speed up review and merge. You can put the change of location modification into another PR

Done: #16438

wojtuss · 2019-03-25T12:17:02Z

@luotao1 , #15987 (comment) is also done

luotao1 · 2019-03-25T12:28:53Z

paddle/fluid/inference/api/analysis_predictor.h

+  class Quantizer;
+  Quantizer *quantizer_{nullptr};
+
+  friend class QuantizerTest;


please remove friend class QuantizerTest in analysis_predictor.h. Header file doesn't need to contain a test class.

The Quantizer class is a private nested class, and we need the friendship declaration to create an instance of Quantizer in the QuantizerTest test (analysis_predictor_tester.cc).

We could remove the declaration by moving the class Quantizer; forward declaration to the public section, but we would like to keep the Quantizer class hidden as an implementation detail and not to expose it to the outside.

@Superjomn How do you think about it?

That's OK I think, but need to wrap by #ifdef PADDLE_WITH_TESTING ? @luotao1

But the Quantize method should be general, or it should be named to MkldnnQuantize

#ifdef PADDLE_WITH_TESTING, be named to MkldnnQuantize

I agree with it @wojtuss @Superjomn

@luotao1 , @Superjomn , to avoid too many updates of the PR we could agree to add the Mkldnn prefix to all "quantize"-related methods, classes, and file names in this PR. What do you think?

all "quantize"-related methods, classes, and file names

Do you mean MkldnnQuantizerTest, EnableMkldnnQuantizer, mkldnn_quantizer.cc,mkldnn_ quantizer_config.cc, use_mkldnn_quantizer_, mkldnn_quantizer_config_, MkldnnQuantizer, mkldnn_quantizer_enabled,etc?

I think it is OK.

@luotao1, done.
I left renaming of cpu_quantize_* passes to another PR.
Do you think it is urgent to have the things in passes renamed asap?

test=develop Co-authored-by: Sylwester Fraczek <sylwester.fraczek@intel.com>

test=develop

luotao1 · 2019-03-26T07:27:27Z

paddle/fluid/inference/api/analysis_config.cc

@@ -143,6 +148,25 @@ void AnalysisConfig::EnableMKLDNN() {
  Update();
 }

+void AnalysisConfig::EnableQuantizer() {


EnableMkldnnQuantizer

luotao1 · 2019-03-26T07:27:49Z

paddle/fluid/inference/api/CMakeLists.txt

@@ -29,13 +29,19 @@ endif()

 add_subdirectory(details)

-cc_library(analysis_config SRCS analysis_config.cc DEPS lod_tensor paddle_pass_builder)
+if(WITH_MKLDNN)
+	set(quantizer_src quantizer.cc)


mkldnn_quantizer.cc?
mkldnn_ quantizer_config.cc?

luotao1 · 2019-03-26T07:30:48Z

paddle/fluid/inference/api/analysis_config.cc

@@ -107,6 +107,11 @@ AnalysisConfig::AnalysisConfig(const AnalysisConfig &other) {
  // MKLDNN related.
  CP_MEMBER(use_mkldnn_);
  CP_MEMBER(mkldnn_enabled_op_types_);
+  // Quantization related.
+  CP_MEMBER(use_quantizer_);


use_mkldnn_quantizer_
mkldnn_quantizer_config_

luotao1 · 2019-03-26T07:31:32Z

paddle/fluid/inference/api/analysis_predictor.h

@@ -141,6 +144,16 @@ class AnalysisPredictor : public PaddlePredictor {
  std::vector<framework::OpDesc *> fetches_;
  std::map<size_t, std::string> idx2fetches_;

+#if PADDLE_WITH_MKLDNN
+  // Helper class to perform quantization
+  class Quantizer;


MkldnnQuantizer

luotao1 · 2019-03-26T07:31:46Z

paddle/fluid/inference/api/analysis_predictor.h

+  class Quantizer;
+  Quantizer *quantizer_{nullptr};
+
+  friend class QuantizerTest;


MkldnnQuantizerTest

luotao1 · 2019-03-26T07:37:39Z

paddle/fluid/inference/api/paddle_analysis_config.h

@@ -174,6 +177,18 @@ struct AnalysisConfig {
    mkldnn_enabled_op_types_ = op_list;
  }

+  /** Turn on quantization.
+   */
+  void EnableQuantizer();


EnableMkldnnQuantizer

luotao1 · 2019-03-26T07:37:51Z

paddle/fluid/inference/api/paddle_analysis_config.h

+
+  /** A boolean state telling whether the quantization is enabled.
+  */
+  bool quantizer_enabled() const { return use_quantizer_; }


mkldnn_quantizer_enabled

… sfraczek/quantization-core3

wojtuss · 2019-03-26T21:59:58Z

@luotao1 , the build PR_CI (Paddle) is failing for a very strange reason:

[16:00:31]	+ ./vis_demo --modeldir=/root/.cache/inference_demo/se_resnext50/model --data=/root/.cache/inference_demo/se_resnext50/data.txt --refer=/root/.cache/inference_demo/se_resnext50/result.txt --use_gpu=true
[16:00:34]	WARNING: Logging before InitGoogleLogging() is written to STDERR
[16:00:34]	W0326 16:00:34.680001  2324 device_context.cc:262] Please NOTE: device: 0, CUDA Capability: 61, Driver API Version: 9.0, Runtime API Version: 8.0
[16:00:34]	W0326 16:00:34.680243  2324 device_context.cc:270] device: 0, cuDNN Version: 7.0.
[16:00:35]	--- Running analysis [ir_graph_build_pass]
[16:00:36]	--- Running analysis [ir_analysis_pass]
[16:00:36]	terminate called after throwing an instance of 'paddle::platform::EnforceNotMet'
[16:00:36]	  what():  Pass infer_clfan_graph_pass has not been registered at [/paddle/paddle/fluid/framework/ir/pass.h:159]

It looks like a typo in the code, but there is nothing like that or I can't find it. Rebuild didn't help.
Have you encountered a similar issue before?
I'll pull the branch to the current develop.

… sfraczek/quantization-core3

luotao1 · 2019-03-27T10:35:30Z

I have a similar error with CI.

cd Paddle/build
make -j
make inference_lib_dist -j
cd Paddle/paddle/fluid/inference/api/demo_ci
/run.sh /home/luotao/Paddle ON OFF /home/luotao/Paddle/paddle/fluid/inference/api/demo_ci/data/

result

-- Generating done
-- Build files have been written to: /home/luotao/Paddle/paddle/fluid/inference/api/demo_ci/build
+ make -j
Scanning dependencies of target vis_demo
[ 50%] Building CXX object CMakeFiles/vis_demo.dir/vis_demo.cc.o
[100%] Linking CXX executable vis_demo
[100%] Built target vis_demo
+ for use_gpu in '$use_gpu_list'
+ for vis_demo_name in '$vis_demo_list'
+ ./vis_demo --modeldir=/home/luotao/Paddle/paddle/fluid/inference/api/demo_ci/data//se_resnext50/model --data=/home/luotao/Paddle/paddle/fluid/inference/api/demo_ci/data//se_resnext50/data.txt --refer=/home/luotao/Paddle/paddle/fluid/inference/api/demo_ci/data//se_resnext50/result.txt --use_gpu=false
./run.sh: line 91: 10951 Segmentation fault      ./vis_demo --modeldir=$DATA_DIR/$vis_demo_name/model --data=$DATA_DIR/$vis_demo_name/data.txt --refer=$DATA_DIR/$vis_demo_name/result.txt --use_gpu=$use_gpu
+ '[' 139 -ne 0 ']'
+ echo 'vis demo se_resnext50 runs fail.'
vis demo se_resnext50 runs fail.
+ exit 1

luotao1 · 2019-03-27T12:00:16Z

Some observations:

The tensorrt models run successfully on CI. Since tensorrt will clean and re-assign passes

Paddle/paddle/fluid/inference/api/analysis_config.cc

Lines 238 to 250 in 54a7357

    
           if (use_anakin_) { 
        
             PADDLE_ENFORCE(!use_tensorrt_, 
        
                            "Anakin sub-graph and TensorRT sub-graph are not allowed to " 
        
                            "run at the same time!"); 
        
             PADDLE_ENFORCE( 
        
                 use_gpu_, 
        
                 "Anakin sub-graph engine need gpu, please use the EnableGpu API."); 
        
             pass_builder()->ClearPasses(); 
        
             for (const auto &pass : kAnakinSubgraphPasses) { 
        
               pass_builder()->AppendPass(pass); 
        
             } 
        
           }

test_analyzer_xxx runs successfully, but only demo_ci runs error. Does it means there are some errors in inference_lib?
I test both shared and static inference_lib, demo_ci runs error on both.

luotao1 · 2019-03-27T12:29:19Z

--- a/paddle/fluid/inference/api/analysis_config.cc
+++ b/paddle/fluid/inference/api/analysis_config.cc
@@ -111,7 +111,7 @@ AnalysisConfig::AnalysisConfig(const AnalysisConfig &other) {
   // Quantization related.
   CP_MEMBER(use_mkldnn_quantizer_);
 #ifdef PADDLE_WITH_MKLDNN
-  CP_MEMBER(mkldnn_quantizer_config_);
+  // CP_MEMBER(mkldnn_quantizer_config_);
 #endif

After I delete this line, I could run successfully demo_ci on local machine.
mkldnn_quantizer_config_ is nullptr at first, could it use CP_MEMBER?

luotao1 · 2019-03-27T12:36:35Z

paddle/fluid/inference/api/analysis_config.cc

@@ -148,6 +153,28 @@ void AnalysisConfig::EnableMKLDNN() {
  Update();
 }

+void AnalysisConfig::EnableMkldnnQuantizer() {
+#ifdef PADDLE_WITH_MKLDNN
+  if (!mkldnn_quantizer_config_)


if delete line158, then we don't need CP_MEMBER(mkldnn_quantizer_config_)? Could we delete line158?

@luotao1 , copying the shared_ptr worked and it should be fine, even when it holds nullptr. I would rather suspect the #ifdef PADDLE_WITH_MKLDNN in the paddle_analysis_config.h is the culprit. I am investigating that now.

@luotao1 , I confirm that removing #ifdef PADDLE_WITH_MKLDNN solved the problem. I took care of it in another way. Please take a look at this patch if it is OK for you: wojtuss@49c5a2b
I am verifying it more thoroughly.
[edit]
It works. I have updated the PR.

Got it. I verify on my local machine.

test=develop

luotao1

If this PR passes all the CI in night, please squash and merge it yourself.

wojtuss · 2019-03-27T19:16:25Z

@luotao1 , thank you very much for your help! 😄 👍

wojtuss added Intel int8 labels Mar 22, 2019

wojtuss requested review from Superjomn and luotao1 March 22, 2019 11:25

wojtuss mentioned this pull request Mar 22, 2019

C-API quantization core #15987

Closed

wojtuss force-pushed the sfraczek/quantization-core3 branch from dc75415 to d8a7969 Compare March 22, 2019 12:03

wojtuss changed the title ~~C-API quantization core~~ C-API quantization core 2 Mar 22, 2019

wojtuss force-pushed the sfraczek/quantization-core3 branch 2 times, most recently from bf8f5c3 to 79d8cc3 Compare March 25, 2019 10:47

luotao1 reviewed Mar 25, 2019

View reviewed changes

wojtuss force-pushed the sfraczek/quantization-core3 branch 3 times, most recently from d596d48 to 983017b Compare March 25, 2019 14:36

Wojciech Uss and others added 3 commits March 25, 2019 19:32

$@sfraczek$

C-API quantization core

88916a2

test=develop Co-authored-by: Sylwester Fraczek <sylwester.fraczek@intel.com>

Decouple Quantizer from AnalysisPredictor

a07b3fd

test=develop

fixes after review

9a920ae

test=develop

wojtuss force-pushed the sfraczek/quantization-core3 branch from 983017b to a07b3fd Compare March 26, 2019 01:32

renamed mkldnn quantize stuff

3005bce

test=develop

wojtuss force-pushed the sfraczek/quantization-core3 branch from 50d565e to 9a920ae Compare March 26, 2019 07:29

luotao1 reviewed Mar 26, 2019

View reviewed changes

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

ec6c872

… sfraczek/quantization-core3

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

5b93d82

… sfraczek/quantization-core3

luotao1 reviewed Mar 27, 2019

View reviewed changes

remove ifdef from header file

49c5a2b

test=develop

luotao1 approved these changes Mar 27, 2019

View reviewed changes

luotao1 mentioned this pull request Mar 27, 2019

create test for quantized resnet50 #16399

Merged

wojtuss merged commit 09dfc7a into PaddlePaddle:develop Mar 27, 2019

C-API quantization core 2 #16396

C-API quantization core 2 #16396

Conversation

wojtuss commented Mar 22, 2019

wojtuss commented Mar 25, 2019

luotao1 commented Mar 25, 2019

luotao1 commented Mar 25, 2019

wojtuss commented Mar 25, 2019 • edited

wojtuss commented Mar 25, 2019

Choose a reason for hiding this comment

wojtuss Mar 25, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Superjomn Mar 26, 2019 • edited

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

wojtuss Mar 26, 2019 • edited

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

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

wojtuss commented Mar 26, 2019 • edited

luotao1 commented Mar 27, 2019

luotao1 commented Mar 27, 2019

luotao1 commented Mar 27, 2019 • edited

luotao1 Mar 27, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wojtuss Mar 27, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luotao1 left a comment

Choose a reason for hiding this comment

wojtuss commented Mar 27, 2019

wojtuss commented Mar 25, 2019 •

edited

wojtuss Mar 25, 2019 •

edited

Superjomn Mar 26, 2019 •

edited

wojtuss Mar 26, 2019 •

edited

wojtuss commented Mar 26, 2019 •

edited

luotao1 commented Mar 27, 2019 •

edited

luotao1 Mar 27, 2019 •

edited

wojtuss Mar 27, 2019 •

edited