Skip to content

[Relay] Fix invalid shape function for "copy" operator#9749

Merged
masahi merged 1 commit into
apache:mainfrom
mbs-octoml:mbs-CORE-112
Dec 18, 2021
Merged

[Relay] Fix invalid shape function for "copy" operator#9749
masahi merged 1 commit into
apache:mainfrom
mbs-octoml:mbs-CORE-112

Conversation

@mbs-octoml

@mbs-octoml mbs-octoml commented Dec 15, 2021

Copy link
Copy Markdown
Contributor

The @script shape function for copy was ill-formed, resulting in a TIR shape
function which did not assign to its output. That in turn caused either OOM or
assert fails as uninitialized shape dimensions worked their way downstream.
That fix is in python/tvm/relay/op/tensor.py.

Special thanks to @electriclilies for helping me with the scalar vs tensor switch
in the shape function. It turns out copy is used for scalars in some models.

Everything else is for testing and debugging as I tracked
this down.

[This is CORE-112 in OctoML JIRA.]

@electriclilies electriclilies left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall LGTM! I have a few nitty questions but other than that, good to go in

Comment thread include/tvm/runtime/debug.h
Comment thread src/relay/backend/te_compiler_cache.cc
Comment thread src/target/compilation_config.cc
Comment thread tests/python/relay/dyn/test_dynamic_op_level3.py Outdated
@mbs-octoml

Copy link
Copy Markdown
Contributor Author

Thanks @electriclilies . Here goes another 6 hours of CI!

@mbs-octoml

Copy link
Copy Markdown
Contributor Author

Dang, stepped on my own toes in #9759. Here we go again.

The 'script' for of the shape function was ill-formed,
resulting in a TIR shape function which did not assign
to it's output, which in turn caused either OOM or
assert fails as uninitialized dimensions worked their
way downstream. That fix is in python/tvm/relay/op/tensor.py.

Everything else is for testing and debugging as I tracked
this down.

Special thanks to Lily for helping me with the scalar vs
tensor switch in the copy shape function.

[This is CORE-112 in OctoML JIRA.]
@masahi masahi merged commit 89b1676 into apache:main Dec 18, 2021
@mbs-octoml mbs-octoml deleted the mbs-CORE-112 branch January 3, 2022 19:04
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
The 'script' for of the shape function was ill-formed,
resulting in a TIR shape function which did not assign
to it's output, which in turn caused either OOM or
assert fails as uninitialized dimensions worked their
way downstream. That fix is in python/tvm/relay/op/tensor.py.

Everything else is for testing and debugging as I tracked
this down.

Special thanks to Lily for helping me with the scalar vs
tensor switch in the copy shape function.

[This is CORE-112 in OctoML JIRA.]
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
The 'script' for of the shape function was ill-formed,
resulting in a TIR shape function which did not assign
to it's output, which in turn caused either OOM or
assert fails as uninitialized dimensions worked their
way downstream. That fix is in python/tvm/relay/op/tensor.py.

Everything else is for testing and debugging as I tracked
this down.

Special thanks to Lily for helping me with the scalar vs
tensor switch in the copy shape function.

[This is CORE-112 in OctoML JIRA.]
qsqqsqqsq-intellif pushed a commit to qsqqsqqsq-intellif/tvm that referenced this pull request Apr 29, 2022
The 'script' for of the shape function was ill-formed,
resulting in a TIR shape function which did not assign
to it's output, which in turn caused either OOM or
assert fails as uninitialized dimensions worked their
way downstream. That fix is in python/tvm/relay/op/tensor.py.

Everything else is for testing and debugging as I tracked
this down.

Special thanks to Lily for helping me with the scalar vs
tensor switch in the copy shape function.

[This is CORE-112 in OctoML JIRA.]
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