Skip to content

Conversation

@electriclilies
Copy link
Contributor

@electriclilies electriclilies commented Jan 8, 2022

In this PR, I add some JSON converters. The motivation for this change is #9826-- I need to visit the virtual_device_ field in the AttrVisitor of many Relay nodes. Unfortunately, this breaks importing models serialized with TVM v 0.8. This change allows us to import those models.

I also changed the behavior of the upgrade_json function to update 0.6 to current (0.9), 0.7 to current (0.9) and 0.8 to current (0.9).

@mbs-octoml @tqchen @mbrookhart @mikepapadim PTAL if you are interested! Thanks

Copy link
Member

@tqchen tqchen left a comment

Choose a reason for hiding this comment

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

Thanks @electriclilies , one minor nit and it LGTM

"""

def _initialize_virtual_device(item, _):
if "virtual_device_" not in item["attrs"].keys():
Copy link
Member

Choose a reason for hiding this comment

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

"virtual_device_" not in item["attrs"] is better so it do ont have to instantiate keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Thanks for the review

@masahi masahi merged commit e4d0181 into apache:main Jan 11, 2022
@electriclilies
Copy link
Contributor Author

Thanks @masahi @tqchen!

Raghav-Chakravarthy pushed a commit to Raghav-Chakravarthy/tvm that referenced this pull request Jan 28, 2022
* Add default to serialization

* revert changes in serialization.cc

* update 0.6 converter

* json updater working, except for cycles

* clean up code

* Fix tests

* formatting

* format
:

* nit
ylc pushed a commit to ylc/tvm that referenced this pull request Feb 16, 2022
* Add default to serialization

* revert changes in serialization.cc

* update 0.6 converter

* json updater working, except for cycles

* clean up code

* Fix tests

* formatting

* format
:

* nit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants