You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/07/07 20:12:04 UTC
[GitHub] [incubator-tvm] mbrookhart opened a new pull request #6008: Dynamic TopK Op
mbrookhart opened a new pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008
@zhiics @lixiaoquan @kevinthesun
This splits dynamic TopK out of the standard op, many thanks to @kevinthesun for all of the pieces. :)
Adds the op to the dynamic to static pass, and adds dynamic testing.
Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452378555
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -33,7 +34,9 @@ namespace relay {
class DynamicToStaticMutator : public MixedModeMutator {
public:
DynamicToStaticMutator()
- : dyn_reshape_op_(Op::Get("dyn.reshape")), dyn_tile_op_(Op::Get("dyn.tile")) {}
+ : dyn_reshape_op_(Op::Get("dyn.reshape")),
Review comment:
The ops are actually cached in a global registry. This caching here is to reduce the dynamic lookup overhead. So using Get should be okay as well
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452369212
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -55,6 +58,20 @@ class DynamicToStaticMutator : public MixedModeMutator {
static const Op& op = Op::Get("tile");
return Call(op, {call_node->args[0]}, Attrs(attrs), {});
}
+ } else if (call_node->op == dyn_topk_op_) {
+ if (const ConstantNode* k = call_node->args[1].as<ConstantNode>()) {
Review comment:
Maybe we should have `MakeTopK`, `MakeTile`, etc?
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -33,7 +34,9 @@ namespace relay {
class DynamicToStaticMutator : public MixedModeMutator {
public:
DynamicToStaticMutator()
- : dyn_reshape_op_(Op::Get("dyn.reshape")), dyn_tile_op_(Op::Get("dyn.tile")) {}
+ : dyn_reshape_op_(Op::Get("dyn.reshape")),
Review comment:
In this case, I think it is probably okay to not cache them. Otherwise, we may have a long list here and a long list of members. This would have some lookup overhead, but it increases the readability. How do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#issuecomment-656815967
Thanks @mbrookhart @lixiaoquan @kevinthesun
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452387387
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -33,7 +34,9 @@ namespace relay {
class DynamicToStaticMutator : public MixedModeMutator {
public:
DynamicToStaticMutator()
- : dyn_reshape_op_(Op::Get("dyn.reshape")), dyn_tile_op_(Op::Get("dyn.tile")) {}
+ : dyn_reshape_op_(Op::Get("dyn.reshape")),
Review comment:
no problem. Maybe do a cleanup PR with the MakOp stuff. I notice the duplicate as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
lixiaoquan commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r451980232
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
What I meant is `data`, not `k`. `data` can only be Relay Expr. Should we always use _dyn.topk and let DynmaicToStatic determine whether can convert it to static op?
Because TopK's out_shape depends on shape of data, if shape of data is dynamic, out_shape becomes dynamic, we need to use shape function in that case
https://github.com/apache/incubator-tvm/blob/eafb2aa13d6cd223629f17d5f6aab5a8d4fce7f5/src/relay/op/algorithm/topk.cc#L50
```
shape = infer_shape(data)
if Any_in_shape():
# go dynamic
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r451990004
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
Current dyn namespace is for attrs to be Relay Expr which makes shape func to be data dependent. For data independent shape func we can still relay on "static" version op.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r451990004
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
Current dyn namespace is for attrs to be Relay Expr which makes shape func data dependent. For data independent shape func we can still relay on "static" version op.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452335179
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
Yeah, I think this is a bit of complexity around how @icemelon9 and co implemented dynamic shapes in Relay. Basically, any op can take in `Any` shapes, at which point the graph becomes un-runable on the graph runtime and has to be executed through the VM.
As @kevinthesun said, what we've been working on recently is allowing for non-constant attributes, the `dyn` namespace is mostly to make non-constant attributes explicit. If we need to separate all dynamic shapes, I think we'd need to implement a `vm` specific op namespace, and I'm not sure we want to go that far?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452519137
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
@lixiaoquan Data independent shape func is still there and can handle dynamic input shape cases. @mbrookhart I think for now we can limit dyn namespace to Expr attrs and later consider how to have a more uniform interface.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452519137
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
@lixiaoquan Data independent shape func is still there and can handle dynamic input shape cases. I think for now we can limit dyn namespace to Expr attrs and later consider how to have a more uniform interface.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452387387
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -33,7 +34,9 @@ namespace relay {
class DynamicToStaticMutator : public MixedModeMutator {
public:
DynamicToStaticMutator()
- : dyn_reshape_op_(Op::Get("dyn.reshape")), dyn_tile_op_(Op::Get("dyn.tile")) {}
+ : dyn_reshape_op_(Op::Get("dyn.reshape")),
Review comment:
no problem. Thanks. Maybe do a cleanup PR with the MakOp stuff. I notice the duplicate as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
lixiaoquan commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r451914295
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
I'm concered about this case: If data's shape is not static, it needs to use _dyn_make.topk() too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
lixiaoquan commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r451997081
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
> Current dyn namespace is for attrs to be Relay Expr which makes shape func data dependent. For data independent shape func we can still relay on "static" version op.
I understand that. But it's possbile that an op is input shape depandeant but input shape itself is dynamic. In this kind case, we still need to use shape function.
This is a simliar case which doesn't have topk, but has similar issue.
```
v = var("v", int32)
t0 = arange(0, v, 1) # output an dynamic shape tensor
t1 = strided_slice(t0, [0], [-1], [1]) # output shape depends on a dynamic input shape, even if attrs are static
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452378555
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -33,7 +34,9 @@ namespace relay {
class DynamicToStaticMutator : public MixedModeMutator {
public:
DynamicToStaticMutator()
- : dyn_reshape_op_(Op::Get("dyn.reshape")), dyn_tile_op_(Op::Get("dyn.tile")) {}
+ : dyn_reshape_op_(Op::Get("dyn.reshape")),
Review comment:
The ops are actually cached in a global registry. This caching here is to reduce the dynamic lookup overhead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452384523
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -33,7 +34,9 @@ namespace relay {
class DynamicToStaticMutator : public MixedModeMutator {
public:
DynamicToStaticMutator()
- : dyn_reshape_op_(Op::Get("dyn.reshape")), dyn_tile_op_(Op::Get("dyn.tile")) {}
+ : dyn_reshape_op_(Op::Get("dyn.reshape")),
Review comment:
Next PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452386661
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -55,6 +58,20 @@ class DynamicToStaticMutator : public MixedModeMutator {
static const Op& op = Op::Get("tile");
return Call(op, {call_node->args[0]}, Attrs(attrs), {});
}
+ } else if (call_node->op == dyn_topk_op_) {
+ if (const ConstantNode* k = call_node->args[1].as<ConstantNode>()) {
Review comment:
There are a lot of duplicated or referenced MakeOp functions in here: https://github.com/apache/incubator-tvm/blob/master/src/relay/transforms/pattern_util.h, I'll make a separate PR to pull those out into an op-level utility header and clean things up a bit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#issuecomment-656489932
@lixiaoquan @kevinthesun please https://tvm.apache.org/docs/contribute/code_review.html#approve-and-request-changes-explicitly
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
lixiaoquan commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452579414
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
> Thanks for pointing that out, I just realize 'static' version op still have some dynamic shape handling ability. I think I misunderstood 'dyn' namespace before.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics merged pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
zhiics merged pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452375570
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -33,7 +34,9 @@ namespace relay {
class DynamicToStaticMutator : public MixedModeMutator {
public:
DynamicToStaticMutator()
- : dyn_reshape_op_(Op::Get("dyn.reshape")), dyn_tile_op_(Op::Get("dyn.tile")) {}
+ : dyn_reshape_op_(Op::Get("dyn.reshape")),
Review comment:
I'm thinking a Registry will be the best combination, we can do a dynamic lookup to see if the op is registered, simplify this codebase, and still have the caching optimization.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
lixiaoquan commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452579368
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
Thanks for pointing that out, I just realize 'static' version op still have some dynamic shape handling ability. I think I misunderstood 'dyn' namespace before.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r452370146
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -55,6 +58,20 @@ class DynamicToStaticMutator : public MixedModeMutator {
static const Op& op = Op::Get("tile");
return Call(op, {call_node->args[0]}, Attrs(attrs), {});
}
+ } else if (call_node->op == dyn_topk_op_) {
+ if (const ConstantNode* k = call_node->args[1].as<ConstantNode>()) {
Review comment:
It's an idea, those functions are available, we'd just have to pull them into headers.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #6008: [Relay][Dyn] Dynamic TopK Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6008:
URL: https://github.com/apache/incubator-tvm/pull/6008#discussion_r451928744
##########
File path: python/tvm/relay/op/algorithm.py
##########
@@ -82,9 +84,12 @@ def topk(data, k=1, axis=-1, ret_type="both",
out : relay.Expr or List[relay.Expr]
The computed result.
"""
- if isinstance(k, int):
- k = const(k, "int64")
- out = _make.topk(data, k, axis, ret_type, is_ascend, dtype)
+ if isinstance(k, Constant):
+ k = np.asscalar(k.data.asnumpy())
+ if isinstance(k, Expr):
Review comment:
I might be missing something. If it's a Relay Constant, we go static. If it's another Relay Expr, we go dynamic, if it's a python literal, we go static. Is there another possibility?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org