Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable python debug runtime for exported network libraries #8793

Merged
merged 7 commits into from
Sep 2, 2021
Merged

Enable python debug runtime for exported network libraries #8793

merged 7 commits into from
Sep 2, 2021

Conversation

elvin-n
Copy link
Contributor

@elvin-n elvin-n commented Aug 19, 2021

Python debug runtime object require json for its initialization, but in case of previously exported library, json is not accessible outside of the compiled module. Added PAcked func "get_json" to module and updated documentation how to init python debug_executor for this case

tkonolige
tkonolige requested changes Aug 19, 2021
View reviewed changes
docs/dev/debugger.rst Outdated

::

# Whether enable additional graph debug functions
set(USE_GRAPH_EXECUTOR_DEBUG ON)
set(USE_GRAPH_RUNTIME_DEBUG ON)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

USE_GRAPH_EXECUTOR_DEBUG and USE_GRAPH_RUNTIME_DEBUG are deprecated. You should use USE_PROFILER instead.

docs/dev/debugger.rst Outdated
Comment on lines 157 to 158
m = graph_executor.GraphModuleDebug(lib["debug_create"]("default", dev),
[dev], lib["get_json"](), dump_root="/tmp/tvmdbg")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does graph_executor.create not work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please share how to pass lib to the second param of graph_executor.create? If I pass lib been created through tvm.runtime.load_module directly, it does not work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

graph_executor.create(lib["get_json"](), lib, dev, dump_root="/tmp/tvmdbg") seems to work for me locally. Maybe I am testing it wrong though?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, interesting. I tried the same code and it did not work, but when I tried your code snippet, everything started to work well. Will update the doc. I might not be required, but still there is a difference how to get the json between just compiled lib and loaded from the external dynamic library -lib["get_json"]()

@tkonolige
Copy link
Contributor

Could you rename get_json to get_graph_json to match the graph executor codegen? And could you also add a test?

@elvin-n
Copy link
Contributor Author

elvin-n commented Aug 25, 2021

@tkonolige addressed comments

tkonolige
tkonolige approved these changes Aug 25, 2021
View reviewed changes
Copy link
Contributor

@tkonolige tkonolige left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, except for a little change on the tests.

tests/python/unittest/test_runtime_module_based_interface.py Outdated
Comment on lines 93 to 96
def test_cpu_get_graph_json():
if not tvm.testing.device_enabled("llvm"):
print("Skip because llvm is not enabled")
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def test_cpu_get_graph_json():
if not tvm.testing.device_enabled("llvm"):
print("Skip because llvm is not enabled")
return
@tvm.testing.requires_llvm
def test_cpu_get_graph_json():

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed together with other tests in this script

@elvin-n elvin-n force-pushed the amalyshe/json_lib branch 5 times, most recently from f86d00b to 92ba85b Compare August 27, 2021 13:32
@jwfromm jwfromm merged commit 707c4e0 into apache:main Sep 2, 2021
@jwfromm
Copy link
Contributor

jwfromm commented Sep 2, 2021

Thanks @elvin-n and @tkonolige. This is now merged.

AndrewZhaoLuo added a commit to AndrewZhaoLuo/tvm that referenced this pull request Sep 2, 2021
@AndrewZhaoLuo
Merge branch 'main' into aluo/onnx/sceloss
4e43297 
* main:
  [UnitTests][Contrib] Enable contrib tensorrt/coreml unit tests (apache#8902)
  [BUG] DataType Bug In SplitRel (apache#8899)
  Enable python debug runtime for exported network libraries (apache#8793)
  Set default value of p in LpPool as 2 (apache#8866)
  [Community] @Hzfengsy -> Committer (apache#8908)
  Trivial uTVM -> microTVM "spelling" fix to align with branding. (apache#8905)
  [Vulkan][Topi] Parametrizing additional topi tests, marking vulkan failures (apache#8904)
  Move to new style issue template system (apache#8898)
  [Onnx] Support Negative Log Loss (apache#8872)
  [ROCm][TVMC] Add ROCm to the TVMC driver (apache#8896)
  fix error report on Store (apache#8895)
  [Docker] Re-enabled automatic --tty flag when running bash. (apache#8861)
@elvin-n elvin-n deleted the amalyshe/json_lib branch September 23, 2021 08:25
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
@elvin-n @apeskov @ylc
Enable python debug runtime for exported network libraries (apache#8793)
f8a4b2f 
* Add get_json method to graph_eceutor factory

Signed-off-by: Alexander Peskov <peskovnn@gmail.com>

* Update Debugger runtime documentation for exported libraries

* Fix cpplint

* Change module get_json to get_graph_json, add test

* Fix get_graph_json test

* Change verificatino of llvm support in tet to decorator

* Fix sphinx warning in debugger.rst

Co-authored-by: Alexander Peskov <peskovnn@gmail.com>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@elvin-n @apeskov @ylc
Enable python debug runtime for exported network libraries (apache#8793)
2949639 
* Add get_json method to graph_eceutor factory

Signed-off-by: Alexander Peskov <peskovnn@gmail.com>

* Update Debugger runtime documentation for exported libraries

* Fix cpplint

* Change module get_json to get_graph_json, add test

* Fix get_graph_json test

* Change verificatino of llvm support in tet to decorator

* Fix sphinx warning in debugger.rst

Co-authored-by: Alexander Peskov <peskovnn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkonolige tkonolige tkonolige approved these changes

@jwfromm jwfromm jwfromm approved these changes

@areusch areusch Awaiting requested review from areusch

@comaniac comaniac Awaiting requested review from comaniac

@jroesch jroesch Awaiting requested review from jroesch

@junrushao junrushao Awaiting requested review from junrushao

@kazum kazum Awaiting requested review from kazum

@liangfu liangfu Awaiting requested review from liangfu

@masahi masahi Awaiting requested review from masahi

@merrymercy merrymercy Awaiting requested review from merrymercy

@tmoreau89 tmoreau89 Awaiting requested review from tmoreau89

@tqchen tqchen Awaiting requested review from tqchen

@vinx13 vinx13 Awaiting requested review from vinx13

@yzhliu yzhliu Awaiting requested review from yzhliu

@ZihengJiang ZihengJiang Awaiting requested review from ZihengJiang

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@elvin-n @tkonolige @jwfromm @apeskov