[tvmc] Introduce 'run' subcommand (part 4/4) #6578

leandron · 2020-09-28T16:21:13Z

Introduces the tvmc run subcommand on tvmc, the aims to enable execution of compiled modules using tvmc compile:

Include options to locally or remotelly using RPC
Include support to cpu and gpu devices
Introduces a new function to parse RPC command line strings (will be backported to tvmc tune in a future PR)

cc @comaniac @FrozenGene for reviews

python/tvm/driver/tvmc/common.py

python/tvm/driver/tvmc/runner.py

tests/python/driver/tvmc/test_runner.py

python/tvm/driver/tvmc/runner.py

tests/python/driver/tvmc/conftest.py

comaniac

Almost there.

python/tvm/driver/tvmc/runner.py

tests/python/driver/tvmc/conftest.py

tests/python/driver/tvmc/test_runner.py

* Add 'tvmc run' subcommand to execute compiled modules * Include options to locally or remotelly using RPC * Include support to cpu and gpu devices Co-authored-by: Marcus Shawcroft <marcus.shawcroft@arm.com> Co-authored-by: Matthew Barrett <matthew.barrett@arm.com>

leandron · 2020-10-01T13:50:55Z

something went wrong with CI. Retriggering it now

FrozenGene · 2020-10-01T16:23:12Z

python/tvm/driver/tvmc/runner.py

+        action="store_true",
+        help="generate profiling data from the runtime execution. "
+        "Using --profile requires the Graph Runtime Debug enabled on TVM. "
+        "Profiling may also have an impact on inference time, "


I am a little curious about this sentence. As far as I know, debug graph runtime is just to record op execution time, no other debug flags to add to build tvm(like -g). So how does it affect the inference time?

Even running debug runtime would affect the inference time compared to the graph runtime.

That is a good question. @comaniac can you comment? Maybe this needs some rephrasing?

Yeah I'm just saying the overhead of collecting/dumping op execution time.

However, this overhead shouldn’t affect the inferece time in my opinion. Because we record this using chrono, when we dump, the time is recorded completely and shouldn’t affect our record.

If we want to describle the overhead, maybe we shouldn’t use the inference time and to say we will introduce the overload time of collecting / dumping which will make our pipiline longer than before.

IMO this CLI is facing to end-users more than the developers, so I supposed they care the end-to-end time more than just the inference time. However, I agree with you that rephrasing the description to avoid inference time might be more accurate.

OHTH, given that this PR is in the list of 0.7 release, would you mind letting the PR in first and fix this issue in the follow-up PR along with other TODOs, since the CI takes a long time?

Yeah, of course.

python/tvm/driver/tvmc/runner.py

FrozenGene · 2020-10-01T16:35:20Z

tests/python/driver/tvmc/test_runner.py

+    assert len(sut[1]) == expected_number_of_results_per_line
+
+
+def test_run_tflite_module__with_profile__valid_input(


why there are many function names using double underscore?

This is used on other test cases, mostly to mark separation of sentences. I can revert it back in case it looks too wrong, but for me they are very useful when reading it in a long list of tests with similar names and slightly different conditions.

I suggest we keeping the style like all other test. Current function names are like generated by automatic tools. How about this suggestion?

FrozenGene

Overal LGTM. But we should complete other TODOs in the following prs as this tool is very critial and maybe the most important end userface developers use TVM in the future.

comaniac · 2020-10-02T02:26:28Z

Thanks @leandron @FrozenGene @zhiics @mbaret @tqchen

* [tvmc] Introduce 'run' subcommand (part 4/4) * Add 'tvmc run' subcommand to execute compiled modules * Include options to locally or remotelly using RPC * Include support to cpu and gpu devices Co-authored-by: Marcus Shawcroft <marcus.shawcroft@arm.com> Co-authored-by: Matthew Barrett <matthew.barrett@arm.com> * adjust based on code review comments * make test fixture to safely skip environments without tflite * make --help option more clear * improve error message to show expected inputs * code-review adjusts * update doc-string to default zeros->random Co-authored-by: Marcus Shawcroft <marcus.shawcroft@arm.com> Co-authored-by: Matthew Barrett <matthew.barrett@arm.com>

leandron mentioned this pull request Sep 28, 2020

[RFC] v0.7 Release Planning #6421

Closed

comaniac requested changes Sep 28, 2020

View reviewed changes

FrozenGene requested changes Sep 29, 2020

View reviewed changes

FrozenGene requested changes Sep 30, 2020

View reviewed changes

tests/python/driver/tvmc/conftest.py Show resolved Hide resolved

leandron mentioned this pull request Sep 30, 2020

[tvmc][docs] Getting started tutorial for TVMC #6597

Merged

comaniac requested changes Sep 30, 2020

View reviewed changes

python/tvm/driver/tvmc/runner.py Outdated Show resolved Hide resolved

python/tvm/driver/tvmc/runner.py Show resolved Hide resolved

tests/python/driver/tvmc/conftest.py Show resolved Hide resolved

tests/python/driver/tvmc/test_runner.py Outdated Show resolved Hide resolved

leandron and others added 5 commits October 1, 2020 10:57

adjust based on code review comments

fd56dab

make test fixture to safely skip environments without tflite

cbd837f

make --help option more clear

d6781f4

improve error message to show expected inputs

5e36328

code-review adjusts

5c9d850

comaniac approved these changes Oct 1, 2020

View reviewed changes

FrozenGene reviewed Oct 1, 2020

View reviewed changes

python/tvm/driver/tvmc/runner.py Outdated Show resolved Hide resolved

FrozenGene reviewed Oct 1, 2020

View reviewed changes

update doc-string to default zeros->random

80c36da

FrozenGene approved these changes Oct 1, 2020

View reviewed changes

ZihengJiang mentioned this pull request Oct 2, 2020

TVM v0.7 Release Note Candidate #6486

Closed

comaniac merged commit 5db80f0 into apache:master Oct 2, 2020

comaniac added the status: accepted label Oct 2, 2020

leandron deleted the tvmc_run_feature branch October 7, 2020 12:56

leandron mentioned this pull request Feb 4, 2021

[TVMC] TODO items on TVMC backlog #6792

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tvmc] Introduce 'run' subcommand (part 4/4) #6578

[tvmc] Introduce 'run' subcommand (part 4/4) #6578

leandron commented Sep 28, 2020 •

edited

comaniac left a comment

leandron commented Oct 1, 2020

FrozenGene Oct 1, 2020

comaniac Oct 1, 2020

leandron Oct 1, 2020 •

edited

comaniac Oct 1, 2020

FrozenGene Oct 1, 2020

comaniac Oct 1, 2020

FrozenGene Oct 1, 2020

FrozenGene Oct 1, 2020

leandron Oct 1, 2020

FrozenGene Oct 1, 2020

FrozenGene left a comment

comaniac commented Oct 2, 2020

		assert len(sut[1]) == expected_number_of_results_per_line


		def test_run_tflite_module__with_profile__valid_input(

[tvmc] Introduce 'run' subcommand (part 4/4) #6578

[tvmc] Introduce 'run' subcommand (part 4/4) #6578

Conversation

leandron commented Sep 28, 2020 • edited

comaniac left a comment

Choose a reason for hiding this comment

leandron commented Oct 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leandron Oct 1, 2020 • 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

FrozenGene left a comment

Choose a reason for hiding this comment

comaniac commented Oct 2, 2020

leandron commented Sep 28, 2020 •

edited

leandron Oct 1, 2020 •

edited