Decorator 4 disable recursive boxing call #5796

lixinqi · 2021-08-08T10:46:11Z

本pr修复一个关键BUG: 递归调用eager consistent op interpreter时不再设置consistent id，因为此时不是logical op。
本pr加入更强的检查，递归调用eager consistent op interpreter时，禁止boxing行为。

…t_group

…neflow into broadcast_consistent_shape

…r checking consistent tensor meta.

…interpreter.

daquexian · 2021-08-13T01:50:03Z

oneflow/core/job/rank_group.cpp

 #include "oneflow/core/rpc/include/global_process_ctx.h"

 namespace oneflow {

 /*static*/ Maybe<Symbol<RankGroup>> RankGroup::New(Symbol<ParallelDesc> parallel_desc) {
+  return DECORATE(&RankGroup::RawNew, ThreadLocal)(parallel_desc);


从 RawNew(Symbol parallel_desc) 的实现来看它并不需要缓存？直接调 New(const std::set<int64_t>& ranks) 就可以，然后 New(const std::set<int64_t>& ranks) 缓存了 RawNew(const std::set<int64_t>& ranks) 的结果

构建和查询std::set<int64_t>成本也不低。Symbol的成本只有int64_t

daquexian · 2021-08-13T01:52:57Z

python/oneflow/test/modules/test_check_meta_consistency.py

+        sbp = (flow.sbp.broadcast,)
+        y = x.to_consistent(placement=placement, sbp=sbp)
+        y.check_meta_consistency()
+        print(y.shape, y.placement, y.sbp)


这个 print 如果是为了检查正确性，需要改成 test_case.assertTrue(...)

嗯。这应该是忘删了。

daquexian · 2021-08-13T02:03:10Z

oneflow/core/framework/transport_util.cpp

@@ -74,7 +72,6 @@ template<Maybe<void> (*SendOrRecv)(const TransportToken&, int64_t, void*, std::s
                                                   std::function<void()>*)>
 Maybe<void> AccessToAllOtherRanks(Symbol<RankGroup> rank_group, const TransportToken& token,
                                  AsyncTransportCtx* ctx) {
-  CHECK_OR_RETURN(rank_group->ContainingCurrentRank());


这里的检查为什么移动到了外层函数里呢，放在这里会更合理吧

这里发生过BUG。某些情况下rank0 会给 rank{1, 2, 3}发数据，显然rank{1, 2, 3}不会包含rank0

也许我应该改一下函数名称

daquexian · 2021-08-13T02:19:28Z

oneflow/core/framework/tensor_consistent_id.h

+    ++*recursive_depth;
+    RetT&& ret = func(arg0, arg1, outputs, args...);
+    --*recursive_depth;
+    if (*recursive_depth == 0) { JUST(InitConsistentId(outputs)); }


建议：这个地方可以考虑用 raii？这样可以直接 return func(...)，并且之后如果 40 行的区域里出现会提前返回的情况（比如 RetT&& ret = func(arg0, JUST(arg1), outputs, args...);）也能保证这里一定执行

daquexian · 2021-08-13T02:22:44Z

oneflow/core/framework/tensor_rpc_util.h

+struct CheckConsistentTensorMeta<RetT, const std::shared_ptr<one::Tensor>&, Args...> {
+  static_assert(is_maybe<RetT>::value, "returned value type must be Maybe<T>.");
+  template<RetT (*func)(const std::shared_ptr<one::Tensor>&, Args...)>
+  static RetT Call(const std::shared_ptr<one::Tensor>& tensor, Args... args) {


这个函数可以直接返回 CheckConsistentTensorMeta<RetT, const one::Tensor&, Args...>::Call(*tensor, args...) 吗

chengtbf · 2021-08-13T04:43:45Z

mark。暂时移除 automerge，两个小时以后加回来。

github-actions · 2021-08-13T07:49:29Z

Speed stats:

GPU Name: GeForce GTX 1080 

PyTorch resnet50 time: 140.5ms (= 7026.4ms / 50, input_shape=[16, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 128.3ms (= 6414.0ms / 50, input_shape=[16, 3, 224, 224], backward is enabled)
Relative speed: 1.10 (= 140.5ms / 128.3ms)

PyTorch resnet50 time: 82.4ms (= 4120.3ms / 50, input_shape=[8, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 74.3ms (= 3717.0ms / 50, input_shape=[8, 3, 224, 224], backward is enabled)
Relative speed: 1.11 (= 82.4ms / 74.3ms)

PyTorch resnet50 time: 58.3ms (= 2914.2ms / 50, input_shape=[4, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 49.3ms (= 2465.7ms / 50, input_shape=[4, 3, 224, 224], backward is enabled)
Relative speed: 1.18 (= 58.3ms / 49.3ms)

PyTorch resnet50 time: 48.9ms (= 2447.3ms / 50, input_shape=[2, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 44.6ms (= 2229.1ms / 50, input_shape=[2, 3, 224, 224], backward is enabled)
Relative speed: 1.10 (= 48.9ms / 44.6ms)

PyTorch resnet50 time: 45.0ms (= 2251.4ms / 50, input_shape=[1, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 39.1ms (= 1952.9ms / 50, input_shape=[1, 3, 224, 224], backward is enabled)
Relative speed: 1.15 (= 45.0ms / 39.1ms)

lixinqi and others added 14 commits August 6, 2021 16:43

GetBroadcastGroup

9f69d65

Merge branch 'master' of github.com:Oneflow-Inc/oneflow into broadcas…

975c19d

…t_group

fix comment typo.

7fd7e70

broadcast shape and dtype

35d68a3

merge master

61a7e1d

1) rm THREAD_LOCAL_CACHED; 2) fix bugs in ThreadLocal

ddde91b

Merge branch 'master' into broadcast_consistent_shape

629cd25

Merge branch 'master' into broadcast_consistent_shape

c01f290

fix wrong use of LocalRank

2562553

Merge branch 'broadcast_consistent_shape' of github.com:Oneflow-Inc/o…

7e9cb2a

…neflow into broadcast_consistent_shape

Merge branch 'master' into broadcast_consistent_shape

425046d

Merge branch 'master' into broadcast_consistent_shape

3d2ad95

1) a decorator for disabling recursive boxing call; 2) a decorator fo…

c5f3684

…r checking consistent tensor meta.

don't set consistent_id when recursively calling eager consistent op …

32004fe

…interpreter.

lixinqi requested a review from daquexian August 8, 2021 10:47

lixinqi added automerge enhancement system labels Aug 8, 2021

lixinqi added 5 commits August 13, 2021 08:29

merge master

c76c7de

refactor tensor_rpc_util.h

1407230

add GlobalProcessCtx::NodeId and GetParallelId4CurrentProcessCtx

86ed5fd

fix compiler complain

c4b4cd5

fix compiler complain

918c7e2

daquexian reviewed Aug 13, 2021

View reviewed changes

address pr comments

08e8310

daquexian approved these changes Aug 13, 2021

View reviewed changes

Merge branch 'master' into decorator_4_disable_recursive_boxing_call

2fa9966

oneflow-ci-bot self-requested a review August 13, 2021 03:52

chengtbf removed the automerge label Aug 13, 2021

chengtbf removed the request for review from oneflow-ci-bot August 13, 2021 04:43

chengtbf added the automerge label Aug 13, 2021

Merge branch 'master' into decorator_4_disable_recursive_boxing_call

c857061

chengtbf requested a review from oneflow-ci-bot August 13, 2021 06:35

oneflow-ci-bot merged commit c071635 into master Aug 13, 2021

oneflow-ci-bot deleted the decorator_4_disable_recursive_boxing_call branch August 13, 2021 07:54

chengtbf mentioned this pull request Aug 13, 2021

Broadcast consistent shape and dtype #5784

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Decorator 4 disable recursive boxing call #5796

Decorator 4 disable recursive boxing call #5796

lixinqi commented Aug 8, 2021

daquexian Aug 13, 2021

lixinqi Aug 13, 2021

daquexian Aug 13, 2021

lixinqi Aug 13, 2021

daquexian Aug 13, 2021

lixinqi Aug 13, 2021

lixinqi Aug 13, 2021

daquexian Aug 13, 2021

lixinqi Aug 13, 2021

daquexian Aug 13, 2021

lixinqi Aug 13, 2021

chengtbf commented Aug 13, 2021

github-actions bot commented Aug 13, 2021

Decorator 4 disable recursive boxing call #5796

Decorator 4 disable recursive boxing call #5796

Conversation

lixinqi commented Aug 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chengtbf commented Aug 13, 2021

github-actions bot commented Aug 13, 2021