-
Notifications
You must be signed in to change notification settings - Fork 825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bug of Multi-Client src tick output order #6221
Conversation
这是一种实现思路,在 task graph 层级处理 src op 的控制连边问题。但感觉按照 xinqi 之前的思路,在 op graph 层级 autotick 机制解决这个问题有一定的好处,如果能把相似的功能都用同一种形式表达是能增加维护性和鲁棒性的。负责控制子图的驱动的一直是 tick 机制,这次 multi-client 引入的新问题如果在 tick 的范畴下解决是最好的,另外在 task graph 去控制 tick 连边其实属于特例,会增加以后的维护成本。 目前 tick 按照功能层级逐级相连,一共4种 tick: SourceTickOp -> TickOp -> SrcSubsetTickOp -> DeviceTickOp,sink 部分则是 DeviceTickOp -> DstSubsetTickOp -> TickOp -> SinkTickOp,还有一个 AccTickOp 负责控制不同频的驱动
multi-client 的问题感觉是引入了新的 src op,之前 src op 的判别都是通过 REGISTER_AUTO_TICK 注册的,属于静态判断,而在 multi-client 下的 src op 应该是动态的,无法静态判断,感觉应该可以通过搜索找到同一个 placement group 下的所有 src op,然后把它们与 DeviceTickOp 连上控制边,来达到控制不同 rank 上的 op 驱动的目的。 |
我推演了一下,好像不能在 OpGraph 上连控制边 😂 ,因为可能会没有效果,见: oneflow/oneflow/core/graph/task_graph.cpp Line 399 in 82b903b
我们在 OpGraph 上连的控制边 (src -> dst),如果在 OpGraph 上可达的话(表示有先后的数据依赖),是不会插入控制边的?而 output 和 input,其实是被 src tick 可达的吧,只是不是被本 rank 上的 src tick 可达(逻辑图可达,物理图其实不可达) |
Got 了,看来暂时没法在 op graph 层面操作这件事情。我仔细想了下,你这种方法应该还是比较安全的,后期不会产生什么隐患。至于机制的统一性,我们可以在实现分布式编译的功能的时候再去设计下这个机制。 |
增加了测试用例,这个分支 Ready 了 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
看了下plan,和实现匹配,我那个pr的4卡测试合并这个执行也正常
loss = pp_out.mean() | ||
loss.backward() | ||
y = x + y | ||
free_out = y.to_consistent(P1, B) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个没有上下文的估计看不出在做什么,这里加注释说明下应该会更清楚
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好的
Speed stats:
|
Speed stats:
|
修复一个非常重要的 nn.Graph 多卡运行的 BUG:
每个 rank 都会有自己的 WaitAndSendIds 和 CallbackNotify Op/TaskNode 作为该 rank 的 唯一 src 和 唯一 sink;同时每个 rank 的 VM 都只会往本 rank 才有的 input/output 、WaitAndSendIds 和 CallbackNotify 发送 JobInstance 作为 RunLazyJob instruction 的实际执行结果, 参考:
oneflow/oneflow/core/eager/lazy_job_instruction_type.cpp
Line 110 in 7382e4a
但是由于我们 Single-Client 迁移到 Multi-Client,整个计算图变成了多 source 的(每个 rank 都自己独立的 src 和 sink),以至于我们在逻辑图上即使连上了正确的控制边,物理图上每个 rank 的 input output 并不完全由自己 rank 的 src 触发,以至于在多卡运行时(尤其是 流水并行非对称 ,且输入输出也不对称),计算图中的本 rank 的 output 已经执行了,本 rank 的虚拟机还没有给 src 发 JobInstance ,此时就会报错 OutputKernel :buffer 为空 的 BUG。
oneflow/oneflow/core/kernel/output_kernel.cpp
Line 45 in 7382e4a
本 PR 通过在 TaskGraph 上,给每个 rank 的 src tick 和 input/output 连控制边 的方式强制使 output kernel 至少需要依赖本 rank 的 src tick 才能执行。
同时修复了 input/ouput callback 比 callback notify 晚执行的 BUG