[TVM EP] code refactor by vvchernov · Pull Request #10655 · microsoft/onnxruntime

Valery Chernov (vvchernov) · 2022-02-24T06:35:38Z

The main idea of the patch is to clean code of TVM EP for simplification of further development:

Create separated classes for compilation and running
Extract model compilation from Runner class
Implement different specialization of Runner for GraphExecutor and VirtualMachine
Reduce the number of conditions operators in complicated algorithms.
Small refactoring for clarification of system behaviour
all TVM EP dependences were covered by onnxruntime::tvm namespace

Alone functionality added is that each fused node has own compiler, earlier there is only one compiler. It should correctly support the mechanism of the responsibility pass between EPs.

…oes not depend on TvmExecutionProvider

…tion from TVMRunner. connect TVMRunnerImpl to TVMRunner

Valery Chernov (vvchernov) · 2022-03-09T12:51:25Z

Hello Xavier Dupré (@xadupre)! Could you see and start CI?

Xavier Dupré (xadupre) · 2022-03-09T13:49:51Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

Xavier Dupré (xadupre) · 2022-03-09T13:50:23Z

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

azure-pipelines · 2022-03-09T13:50:40Z

Azure Pipelines successfully started running 9 pipeline(s).

azure-pipelines · 2022-03-09T13:51:03Z

Azure Pipelines successfully started running 8 pipeline(s).

Valery Chernov (vvchernov) · 2022-03-09T17:33:03Z

Hello Xavier Dupré (@xadupre) could you restart onnxruntime-python-checks-ci-pipeline, it failed due to read timeout, looks like it is occasual issue of CI test which does not depend on the patch

Xavier Dupré (xadupre) · 2022-03-09T17:46:06Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

Xavier Dupré (xadupre) · 2022-03-14T15:26:53Z

+auto TVMCompiler::operator()(const TvmEPOptions& options,
+                             const TVMTensorShapes& input_shapes) -> ModulePtr {
+    if (mod_) {
+        return mod_;


indentation

Xavier Dupré (xadupre) · 2022-03-14T15:27:41Z

+  size_t pos = src.find( ch );
+  size_t initialPos = 0;
+  while( pos != std::string::npos ) {
+      dst.push_back( src.substr( initialPos, pos - initialPos ) );


indentation

Xavier Dupré (xadupre) · 2022-03-14T15:33:31Z

 }

-int TvmExecutionProvider::CreateStateFunc(ComputeContext* context, FunctionState* state) {
+int TvmExecutionProvider::createStateFunc(ComputeContext* context, FunctionState* state) {


The first letter is lower, is it a private method?

Xavier Dupré (xadupre) · 2022-03-14T15:36:34Z

+                     const std::shared_ptr<TvmModule>& mod,
+                     const InputsInfoMap& inputs_info,
+                     const std::vector<DLTensor>& output_tensors) {
+    runner_ = getTVMRunnerImpl(mod, options, inputs_info, output_tensors);


indentation

Valery Chernov (vvchernov) · 2022-03-15T07:34:10Z

Hello Xavier Dupré (@xadupre)! I've tried to fix all indentations in TVM EP code, also TVM hash commit was updated. Do you have any other comments/remarks?

Xavier Dupré (xadupre) · 2022-03-15T08:57:08Z

It is ok. I'm running the test again.

Xavier Dupré (xadupre) · 2022-03-15T08:57:21Z

/azp run Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

Xavier Dupré (xadupre) · 2022-03-15T08:57:35Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

azure-pipelines · 2022-03-15T08:57:55Z

Azure Pipelines successfully started running 7 pipeline(s).

azure-pipelines · 2022-03-15T08:58:22Z

Azure Pipelines successfully started running 9 pipeline(s).

Valery Chernov (vvchernov) · 2022-03-16T05:33:15Z

Good morning Xavier Dupré (@xadupre)! Could you start Windows CPU CI pipelines?

Xavier Dupré (xadupre) · 2022-03-16T10:38:09Z

/azp run Windows CPU CI Pipeline

azure-pipelines · 2022-03-16T10:38:19Z

Azure Pipelines successfully started running 1 pipeline(s).

Valery Chernov (vvchernov) · 2022-03-16T12:51:29Z

Hello Xavier Dupré (@xadupre). CI tests passed successfully

Xavier Dupré (xadupre) · 2022-03-16T17:44:37Z

/azp run Linux TVM CI Pipeline

Xavier Dupré (xadupre) · 2022-03-17T09:36:13Z

Hi Valery Chernov (@vvchernov), I think this PR may have broken Linux TVM CI Pipeline (see #10895). It is possible to look into it?

Valery Chernov (vvchernov) · 2022-03-17T10:02:28Z

Thanks Xavier Dupré (@xadupre), I'll see it

ytaous · 2022-03-17T19:33:43Z

It seems to break the TVM CI pipeline. Is it possible to revert it for now? Or have a quick fix ....
https://dev.azure.com/onnxruntime/onnxruntime/_build?definitionId=189&_a=summary

Valery Chernov (vvchernov) · 2022-03-17T19:56:20Z

ytaous quick fix here

* rename info to options for TVM EP * transfer options processing from TVMExecutionProvider to TVMEPOptions * transfer TVMRunner to separated files * implement TVMCompiler class * replace CompileFunc by TVMCompiler object. update TVMRunner. now it does not depend on TvmExecutionProvider * correct logging of TVM EP options * RunnerImpl, GERunnerImpl and VMRunnerImpl were implemented * add prepareComputeInfo method * remove update_output_shapes flag * embed all TVM EP dependences to tvm namespace. transfer model compilation from TVMRunner. connect TVMRunnerImpl to TVMRunner * refactor compileModel method * small cleaning * separate TVM EP options data store and processing * replace TvmTensorShape by InlinedVector with max_size 5 * correct indentation * update TVM hash Co-authored-by: Valery Chernov <valery.chernov@deelvin.com>

* rename info to options for TVM EP * transfer options processing from TVMExecutionProvider to TVMEPOptions * transfer TVMRunner to separated files * implement TVMCompiler class * replace CompileFunc by TVMCompiler object. update TVMRunner. now it does not depend on TvmExecutionProvider * correct logging of TVM EP options * RunnerImpl, GERunnerImpl and VMRunnerImpl were implemented * add prepareComputeInfo method * remove update_output_shapes flag * embed all TVM EP dependences to tvm namespace. transfer model compilation from TVMRunner. connect TVMRunnerImpl to TVMRunner * refactor compileModel method * small cleaning * separate TVM EP options data store and processing * replace TvmTensorShape by InlinedVector with max_size 5 * correct indentation * update TVM hash Co-authored-by: Valery Chernov <valery.chernov@deelvin.com> (cherry picked from commit 625a1f7)

Valery Chernov (vvchernov) force-pushed the vc/tvm_ep_refactor branch 4 times, most recently from 83de210 to 53968a6 Compare March 3, 2022 18:05

rename info to options for TVM EP

649e510

Valery Chernov (vvchernov) force-pushed the vc/tvm_ep_refactor branch from 3f391b6 to 649e510 Compare March 5, 2022 06:38

transfer options processing from TVMExecutionProvider to TVMEPOptions

26937eb

Valery Chernov (vvchernov) force-pushed the vc/tvm_ep_refactor branch from 3e95891 to 26937eb Compare March 5, 2022 07:31

Valery Chernov added 3 commits March 5, 2022 10:54

transfer TVMRunner to separated files

6fc0c51

implement TVMCompiler class

6b69f1b

replace CompileFunc by TVMCompiler object. update TVMRunner. now it d…

cb22353

…oes not depend on TvmExecutionProvider

Valery Chernov (vvchernov) force-pushed the vc/tvm_ep_refactor branch from 7c5fe78 to cb22353 Compare March 5, 2022 10:09

Valery Chernov added 4 commits March 5, 2022 13:23

correct logging of TVM EP options

1674c65

RunnerImpl, GERunnerImpl and VMRunnerImpl were implemented

84df071

add prepareComputeInfo method

030a6a2

remove update_output_shapes flag

0cdc473

Valery Chernov (vvchernov) force-pushed the vc/tvm_ep_refactor branch from db9c0a9 to ac70584 Compare March 8, 2022 08:02

embed all TVM EP dependences to tvm namespace. transfer model compila…

f7cf1fc

…tion from TVMRunner. connect TVMRunnerImpl to TVMRunner

Valery Chernov (vvchernov) force-pushed the vc/tvm_ep_refactor branch from ac70584 to f7cf1fc Compare March 9, 2022 05:20

Valery Chernov added 2 commits March 9, 2022 12:23

refactor compileModel method

bbe166b

small cleaning

ab4f77e

Valery Chernov (vvchernov) changed the title ~~WIP: [TVM EP] code refactor~~ [TVM EP] code refactor Mar 9, 2022

separate TVM EP options data store and processing

43f7188

Xavier Dupré (xadupre) reviewed Mar 14, 2022

View reviewed changes

Valery Chernov added 2 commits March 15, 2022 09:42

correct indentation

1401c75

update TVM hash

08e77cc

Xavier Dupré (xadupre) approved these changes Mar 15, 2022

View reviewed changes

Xavier Dupré (xadupre) merged commit 625a1f7 into microsoft:master Mar 16, 2022

Valery Chernov (vvchernov) deleted the vc/tvm_ep_refactor branch March 17, 2022 13:07

Conversation

Valery Chernov (vvchernov) commented Feb 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Valery Chernov (vvchernov) commented Mar 9, 2022

Uh oh!

Xavier Dupré (xadupre) commented Mar 9, 2022

Uh oh!

Xavier Dupré (xadupre) commented Mar 9, 2022

Uh oh!

azure-pipelines Bot commented Mar 9, 2022

Uh oh!

azure-pipelines Bot commented Mar 9, 2022

Uh oh!

Valery Chernov (vvchernov) commented Mar 9, 2022

Uh oh!

Xavier Dupré (xadupre) commented Mar 9, 2022

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!

Valery Chernov (vvchernov) commented Mar 15, 2022

Uh oh!

Xavier Dupré (xadupre) commented Mar 15, 2022

Uh oh!

Xavier Dupré (xadupre) commented Mar 15, 2022

Uh oh!

Xavier Dupré (xadupre) commented Mar 15, 2022

Uh oh!

azure-pipelines Bot commented Mar 15, 2022

Uh oh!

azure-pipelines Bot commented Mar 15, 2022

Uh oh!

Valery Chernov (vvchernov) commented Mar 16, 2022

Uh oh!

Xavier Dupré (xadupre) commented Mar 16, 2022

Uh oh!

azure-pipelines Bot commented Mar 16, 2022

Uh oh!

Valery Chernov (vvchernov) commented Mar 16, 2022

Uh oh!

Xavier Dupré (xadupre) commented Mar 16, 2022

Uh oh!

Xavier Dupré (xadupre) commented Mar 17, 2022

Uh oh!

Valery Chernov (vvchernov) commented Mar 17, 2022

Uh oh!

ytaous commented Mar 17, 2022

Uh oh!

Valery Chernov (vvchernov) commented Mar 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Valery Chernov (vvchernov) commented Feb 24, 2022 •

edited

Loading