[DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass #5826
Conversation
|
Hitting an issue with dynamic shapes on GPU with the VM. |
| data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4) | ||
|
|
||
| Special values -2 and -4 from the standard reshape op would introduce dynamic rank | ||
| in this op. Thus, they are not permitted. |
There was a problem hiding this comment.
Current code allows -2 and -4 as constant value. Should frontend check it like
if newshape_inferable() and newshape == (-2) or newshape == (-4):
return _op.reshape()
else:
return _op.dynamic.reshape()
There was a problem hiding this comment.
The Input is a dynamic Expression, I can't check it at graph construction time.
|
Shall we just call the namespace |
|
I agree with @icemelon9 perhaps |
|
👍 Happy to shorten it. |
mbrookhart
force-pushed
the
mbrookhart/dynamic_reshape
branch
from
9453400 to
e31c03c
Compare
|
In fact, I have another thought on this. Should we make the dynamic ops as the default ones whereas having static ops as special cases for better optimization? |
|
@icemelon9 That was my initial suggestion, I think we reached consensus between TQ, Matt and I that we would introduce dynamic and slowly migrate stuff to use them, then introduce static migrate everything to that, and then finally remove the dynamic prefix inorder to ensure a smooth migration without confusion. |
|
cc @kevinthesun @lixiaoquan given that we agreed on the new convention, it would be great if we can also migrate some of the existing dynamic(symbolic) effort towards this direction |
|
cc @icemelon9 please followup and manage this PR, @jroesch @electriclilies @lixiaoquan please https://tvm.apache.org/docs/contribute/code_review.html#approve-and-request-changes-explicitly |
|
@mbrookhart Since we separate the static and dynamic reshape in this pr, we should remove the |
mbrookhart
force-pushed
the
mbrookhart/dynamic_reshape
branch
from
643b115 to
b423200
Compare
|
@icemelon9 @lixiaoquan Per Haichen's request, I removed the dynamic option from the standard reshape and changed the places that used it to use the new purely dynamic op. This touches many more files, could you take another look? |
mbrookhart
force-pushed
the
mbrookhart/dynamic_reshape
branch
from
b423200 to
f87bbee
Compare
mbrookhart
force-pushed
the
mbrookhart/dynamic_reshape
branch
from
4737479 to
2e2c963
Compare
…oStatic Pass (apache#5826) * Dynamic reshape passing tests * Add Dynamic to Static Pass * rename test file to prevent pytest conflicts * fix clang build * add nested dynamic shape test * remove cuda tests until VM supports dynamic shapes * rename namespace from dynamic to dyn * fix lint * fix lint again * Remove incorrect doc strings * remove dynamic behavior from standard reshape * fix some tests * merge dynamic and static interfaces in python * fix missing import * missed a reference to relay.dyn.reshape * fix vta example * respond to review comments
…oStatic Pass (apache#5826) * Dynamic reshape passing tests * Add Dynamic to Static Pass * rename test file to prevent pytest conflicts * fix clang build * add nested dynamic shape test * remove cuda tests until VM supports dynamic shapes * rename namespace from dynamic to dyn * fix lint * fix lint again * Remove incorrect doc strings * remove dynamic behavior from standard reshape * fix some tests * merge dynamic and static interfaces in python * fix missing import * missed a reference to relay.dyn.reshape * fix vta example * respond to review comments
Initial POC for ideas discussed in https://discuss.tvm.ai/t/dynamic-ops-in-relay/6909
@tqchen @jroesch @kevinthesun @lixiaoquan @icemelon9