[Prepare for MultiProcess xpu] unified gen nccl id, refine imperative reducer #30455
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your contribution! |
wangxicoding
force-pushed
the
unified_gen_nccl_id
branch
from
2c2dcd6
to
82dc202
Compare
wangxicoding
requested review from
ForFishes,
hutuxian,
gongweibao and
sandyhouse
sandyhouse
reviewed
Jan 15, 2021
| @@ -12,7 +12,8 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
| See the License for the specific language governing permissions and | |||
| limitations under the License. */ | |||
|
|
|||
| #include "paddle/fluid/operators/collective/gen_nccl_id_op_helper.h" | |||
| #ifdef PADDLE_WITH_NCCL | |||
There was a problem hiding this comment.
如何兼容多种通信库，如NCCL、BKCL，尤其是当多个通信库需要同时使用时怎么保持兼容。
There was a problem hiding this comment.
BKCL和NCCL接口基本一致，这个文件只给类似nccl的使用，后面用了模板
#ifdef PADDLE_WITH_NCCL
INSTANT_TEMPLATE(ncclUniqueId)
#endif
#ifdef PADDLE_WITH_XPU_BKCL
INSTANT_TEMPLATE(bkclUniqueId)
#endif
wangxicoding
force-pushed
the
unified_gen_nccl_id
branch
from
82dc202
to
e5f4e61
Compare
hutuxian
reviewed
Jan 17, 2021
| ->stream(); | ||
| auto comm_stream = | ||
| platform::NCCLCommContext::Instance().Get(ring_id, place_)->stream(); | ||
| auto event = compute_events_[ring_id].get(); |
There was a problem hiding this comment.
add assert for ring_id < compute_events_.size()? Same as WaitComm
There was a problem hiding this comment.
Done. add assert ring_id >=0 and ring_id < compute_events_.size
hutuxian
reviewed
Jan 17, 2021
| int local_rank_{0}; | ||
| std::vector<std::string> trainer_endpoints_{}; | ||
| std::string current_endpoint_{""}; | ||
| // TODO(shenliang03): support multi stream communication |
There was a problem hiding this comment.
Please help shenliang to remove this line
ForFishes
reviewed
Jan 18, 2021
| PADDLE_ENFORCE_CUDA_SUCCESS( | ||
| platform::dynload::ncclGetUniqueId(&(*nccl_ids)[i])); | ||
| } | ||
| } |
There was a problem hiding this comment.
这里是不是也可以和nccl_context.h那里生成nccl id的函数维持成一个。
There was a problem hiding this comment.
理论是的，准备后面再抽象一下，放到nccl helper或者nccl comm里面。
wangxicoding
changed the title
This was referenced Jan 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR types
Others
PR changes
Others
Describe
支持昆仑动态图多卡的准备PR（下个PR提昆仑动态图多卡支持），主要改动：
1、统一动态图和静态图gen_nccl_id，以及bkcl id的广播。方便下个PR支持昆仑动态图多卡以及静态图多进程多卡模式。
2、动态图
ParallelContext抽取到parallel_context.h文件中，方便作为NCCLParallelContext、BKCLParallelContext、GLOOParallelContext等通信库的基类。添加
WaitCompute()和WaitComm()接口，用于抽取通信等待计算、计算等待通信的逻辑，保持Reducer中代码尽量与设备通信库无关。3、调整动态图reducer部分代码，尽量移除与设备相关代码