Rename GraphRuntime to GraphExecutor #7653

areusch · 2021-03-12T18:58:41Z

This PR implements the "GraphRuntime" part of [RFC] Rename GraphRuntime and ilk to e.g. GraphExecutor. It's quite invasive, so it would be great if everyone could look it over. I think this PR is almost entirely targeted find-replace, as shown by the commit history.

one thing I noticed: @u99127 to look at ARM Compute Lib changes, i'm not sure how you prefer to notate ARM compute lib.

@tqchen @jroesch @junrushao1994 @tkonolige @zhiics @manupa-arm @ZihengJiang

areusch · 2021-03-12T18:59:59Z

µTVM note: you will need to nuke build/standalone_crt after this change.

u99127 · 2021-03-12T20:22:27Z

@d-smirnov , @lhutton1 , @mbaret : could you please take a look ?

tkonolige · 2021-03-12T22:26:39Z

CMakeLists.txt

@@ -303,12 +303,12 @@ else()
  list(APPEND COMPILER_SRCS ${STACKVM_RUNTIME_SRCS})
 endif(USE_STACKVM_RUNTIME)

-if(USE_GRAPH_RUNTIME)
+if(USE_GRAPH_EXECUTOR)
  message(STATUS "Build with Graph runtime support...")


tkonolige · 2021-03-12T22:27:02Z

CMakeLists.txt

@@ -326,7 +326,7 @@ if(USE_PROFILER)
  file(GLOB RUNTIME_GRAPH_DEBUG_SRCS src/runtime/graph/debug/*.cc)


missed this

I think this is fine, the var name derives from the path. or do you mean we should fix path to e.g. src/runtime/graph_executor/...?

tkonolige · 2021-03-12T22:29:48Z

docs/install/from_source.rst

@@ -88,7 +88,7 @@ The configuration of TVM can be modified by `config.cmake`.
  - On macOS, for some versions of Xcode, you need to add ``-lc++abi`` in the LDFLAGS or you'll get link errors.
  - Change ``set(USE_CUDA OFF)`` to ``set(USE_CUDA ON)`` to enable CUDA backend. Do the same for other backends and libraries
    you want to build for (OpenCL, RCOM, METAL, VULKAN, ...).
-  - To help with debugging, ensure the embedded graph runtime and debugging functions are enabled with ``set(USE_GRAPH_RUNTIME ON)`` and ``set(USE_GRAPH_RUNTIME_DEBUG ON)``
+  - To help with debugging, ensure the embedded graph executor and debugging functions are enabled with ``set(USE_GRAPH_EXECUTOR ON)`` and ``set(USE_GRAPH_EXECUTOR_DEBUG ON)``


Suggested change

- To help with debugging, ensure the embedded graph executor and debugging functions are enabled with ``set(USE_GRAPH_EXECUTOR ON)`` and ``set(USE_GRAPH_EXECUTOR_DEBUG ON)``

- To help with debugging, ensure the embedded graph executor and debugging functions are enabled with ``set(USE_GRAPH_EXECUTOR ON)`` and ``set(USE_PROFILER ON)``

zhiics

I think we probably need to create an alias for graph_runtime (if they are all replaced by graph_executor) and remove it in the next release for backward compatibility. We can issue a warning to when graph_runtime is used. Otherwise, I think it would break the downstream deployment once they sync with upstream TVM. Please ignore if you have it already.

areusch · 2021-03-18T21:35:54Z

@zhiics good call. I added python backwards compatibility for the create API. I don't know whether we are sensitive to backwards compatibility for the other languages. since the rest are compiled, it seems like we may not need a deprecation warning there (but, I am happy to add one, I am just not sure what would be best). I could also see us doing something slightly different than precedent here given the magnitude of the change.

zhiics · 2021-03-23T06:36:09Z

@areusch yeah, warning may not be needed. I was just trying to make sure that we don't break all the downstream deployment by letting them know that what is going to happen there.

areusch · 2021-03-23T14:44:22Z

@zhiics it's definitely a breaking change, i'm not opposed to the warning or some other way to notify downstream users. I don't know if code or forum or other is the best channel for that--i'm inclined to lean towards code in case forum or elsewhere also has snippets that may become outdated. i'm happy to do whatever you propose--which way do you prefer?

@tqchen @jroesch in case they have thoughts

tqchen · 2021-03-23T23:18:22Z

Given we are pre-1.0 we can just do our best effort when backward compact is possible but not necessary have to spend an extra mile of effort. may not hurt to post a notice to forum as well. e.g. https://discuss.tvm.apache.org/t/notice-tvm-runtime-rpc-upgrade-in-pr7488/9237/5

zhiics · 2021-03-24T00:05:16Z

@zhiics it's definitely a breaking change, i'm not opposed to the warning or some other way to notify downstream users. I don't know if code or forum or other is the best channel for that--i'm inclined to lean towards code in case forum or elsewhere also has snippets that may become outdated. i'm happy to do whatever you propose--which way do you prefer?

@tqchen @jroesch in case they have thoughts

@areusch Thanks. I also prefer code for the the same reason you mentioned.

areusch · 2021-03-24T01:08:15Z

@zhiics I've added Python backwards-compat. Please let me know if you think this is adequate. If so, I think we are ready to merge.

cnv1989 · 2021-03-24T17:41:55Z

apps/android_deploy/app/src/main/java/org/apache/tvm/android/demo/MainActivity.java

-                        mResultView.setText(label);
-                    }
-                    Log.i(TAG, "prediction finished");
+          if (null != graphExecutorModule) {


Why is the indent size here is 2 while everywhere else its 4?

good catch @cnv1989, I had run git-clang-format directly rather than with the script, and it looks like it affected java files. reverted.

zhiics

LGTM. @yzhliu @jroesch @tqchen please also take a look since there are language binding changes as well.

areusch · 2021-03-26T00:19:35Z

@yzhliu @jroesch @tqchen would be great if we can merge this soon as it's easy for this PR to run in to merge conflicts.

tqchen · 2021-03-26T12:20:22Z

oops. sorry there is a merge conflict due to another renaming PR. @areusch can you rebase. will prioritize merging this

Fix graph runtime -> graph executor

Fix graph runtime -> graph executor lint

Following TVM's rename in apache/tvm#7653, update to avoid errors when tvm.contrib.graph_runtime is removed entirely.

tkonolige reviewed Mar 12, 2021

View reviewed changes

zhiics reviewed Mar 16, 2021

View reviewed changes

areusch force-pushed the executor-rename branch from ebbecd3 to 225236b Compare March 18, 2021 21:34

tqchen added status: need review status: need update need update based on feedbacks labels Mar 18, 2021

areusch force-pushed the executor-rename branch 2 times, most recently from b0ff20a to bc9bab8 Compare March 19, 2021 15:35

areusch force-pushed the executor-rename branch 2 times, most recently from 50110b3 to 2f10d74 Compare March 23, 2021 19:26

cnv1989 reviewed Mar 24, 2021

View reviewed changes

areusch force-pushed the executor-rename branch 3 times, most recently from 3ce0117 to f5b48b6 Compare March 25, 2021 00:00

zhiics reviewed Mar 25, 2021

View reviewed changes

zhiics approved these changes Mar 25, 2021

View reviewed changes

cnv1989 approved these changes Mar 25, 2021

View reviewed changes

areusch force-pushed the executor-rename branch from f5b48b6 to acd8db1 Compare March 26, 2021 00:18

areusch force-pushed the executor-rename branch 2 times, most recently from 2bf1d69 to a269414 Compare March 28, 2021 16:46

areusch force-pushed the executor-rename branch from e002487 to bf02d29 Compare March 29, 2021 04:11

areusch added 13 commits March 29, 2021 08:31

graph_runtime -> graph_executor

774fc0a

more renames

3f66176

"graph runtime" -> "graph executor"

4ad96ee

GRAPH_RUNTIME -> GRAPH_EXECUTOR

32b3adb

git-clang-format

bd3271d

graphRuntime -> graphExecutor

bc010de

rename more variables

3831043

backwards compat changes

854edd4

missed one

605b561

git-clang-format

f4a6c3a

black format

9343129

debug_runtime -> debug_executor

d832c2d

black format

40e797b

areusch force-pushed the executor-rename branch from bf02d29 to 40e797b Compare March 29, 2021 15:32

tqchen merged commit 9b43a64 into apache:main Mar 30, 2021

tqchen added status: accepted and removed status: need review status: need update need update based on feedbacks labels Mar 30, 2021

areusch mentioned this pull request Apr 16, 2021

Bring back GraphRuntimeFactory loader for now. #7868

Merged

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021

Rename GraphRuntime to GraphExecutor (apache#7653)

3152778

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021

Rename GraphRuntime to GraphExecutor (apache#7653)

5c4c73e

Fix graph runtime -> graph executor

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021

Rename GraphRuntime to GraphExecutor (apache#7653)

28f9c45

Fix graph runtime -> graph executor lint

trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021

Rename GraphRuntime to GraphExecutor (apache#7653)

e94b6e7

Fix graph runtime -> graph executor lint

dvhg mentioned this pull request Jun 8, 2021

Fix cherry-picked defines in TRT runtime neo-ai/tvm#209

Merged

akmaru mentioned this pull request Jun 9, 2021

Feature/support tvm in dnn fixstars/ion-kit#79

Merged

areusch mentioned this pull request Jun 17, 2021

Rename runtime-config to executor-config and add documentation for Model Library Format #8270

Merged

Lunderberg added a commit to Lunderberg/tvm-vta that referenced this pull request Aug 13, 2021

Renamed graph_runtime references to graph_executor.

06c911e

Following TVM's rename in apache/tvm#7653, update to avoid errors when tvm.contrib.graph_runtime is removed entirely.

Lunderberg mentioned this pull request Aug 13, 2021

Renamed graph_runtime references to graph_executor. apache/tvm-vta#31

Merged

tmoreau89 pushed a commit to apache/tvm-vta that referenced this pull request Aug 19, 2021

Renamed graph_runtime references to graph_executor. (#31)

4f7d249

Following TVM's rename in apache/tvm#7653, update to avoid errors when tvm.contrib.graph_runtime is removed entirely.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename GraphRuntime to GraphExecutor #7653

Rename GraphRuntime to GraphExecutor #7653

areusch commented Mar 12, 2021

areusch commented Mar 12, 2021

u99127 commented Mar 12, 2021

tkonolige Mar 12, 2021

tkonolige Mar 12, 2021

areusch Mar 18, 2021

tkonolige Mar 12, 2021

areusch Mar 18, 2021

zhiics left a comment

areusch commented Mar 18, 2021

zhiics commented Mar 23, 2021

areusch commented Mar 23, 2021

tqchen commented Mar 23, 2021

zhiics commented Mar 24, 2021

areusch commented Mar 24, 2021

cnv1989 Mar 24, 2021 •

edited

Loading

areusch Mar 24, 2021

zhiics left a comment

areusch commented Mar 26, 2021

tqchen commented Mar 26, 2021

		@@ -326,7 +326,7 @@ if(USE_PROFILER)
		file(GLOB RUNTIME_GRAPH_DEBUG_SRCS src/runtime/graph/debug/*.cc)

	- To help with debugging, ensure the embedded graph executor and debugging functions are enabled with ``set(USE_GRAPH_EXECUTOR ON)`` and ``set(USE_GRAPH_EXECUTOR_DEBUG ON)``
	- To help with debugging, ensure the embedded graph executor and debugging functions are enabled with ``set(USE_GRAPH_EXECUTOR ON)`` and ``set(USE_PROFILER ON)``

Rename GraphRuntime to GraphExecutor #7653

Rename GraphRuntime to GraphExecutor #7653

Conversation

areusch commented Mar 12, 2021

areusch commented Mar 12, 2021

u99127 commented Mar 12, 2021

tkonolige Mar 12, 2021

Choose a reason for hiding this comment

tkonolige Mar 12, 2021

Choose a reason for hiding this comment

areusch Mar 18, 2021

Choose a reason for hiding this comment

tkonolige Mar 12, 2021

Choose a reason for hiding this comment

areusch Mar 18, 2021

Choose a reason for hiding this comment

zhiics left a comment

Choose a reason for hiding this comment

areusch commented Mar 18, 2021

zhiics commented Mar 23, 2021

areusch commented Mar 23, 2021

tqchen commented Mar 23, 2021

zhiics commented Mar 24, 2021

areusch commented Mar 24, 2021

cnv1989 Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

areusch Mar 24, 2021

Choose a reason for hiding this comment

zhiics left a comment

Choose a reason for hiding this comment

areusch commented Mar 26, 2021

tqchen commented Mar 26, 2021

cnv1989 Mar 24, 2021 •

edited

Loading