Skip to content
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

[Auto parallel] Make sure the id semantics of every var and op unique #38132

Merged
merged 4 commits into from
Dec 30, 2021

Conversation

aoyulong
Copy link
Contributor

@aoyulong aoyulong commented Dec 14, 2021

PR types

Others

PR changes

Others

Describe

  • Make sure the id of every var desc and op desc be unique.
  • Use the original_id to record the serial desc which the distributed one copies from.
  • Change the access specifier of GenerateId to private.
  • Other changes based on the above modifications.

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@aoyulong aoyulong closed this Dec 29, 2021
@aoyulong aoyulong reopened this Dec 29, 2021
@@ -239,6 +243,10 @@ class Node {
int desc_order_;
int block_id_{-1};

// Store the original id of var desc or op desc.
// Only use this for auto parallel.
uint64_t original_desc_id_{0};
Copy link
Contributor

@chenwhql chenwhql Dec 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果明确该成员及相关构造函数仅用于自动并行的话,有没有可能通过继承实现?仅用于某一功能的成员放到基类中,设计上是不太好的,或者说此类成员在自动并行开发完善的过程中还是否会新增?还会新增多少?

这个可能要根据你们的实际情况来判断,如果确实必须加到全局基类中,我建议命名上直接带上功能信息,比如auto_parallel_id_,明确这一成员的使用是有特定场景要求的,避免大家没看注释的时候困惑或者误用

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

自动并行目前在python侧,采用非侵入式标记program方式,所以直接复用vardesc和opdesc,id和original_id应该是仅有需求,如果还增加新的,的确要考虑继承或者分拆类,另外未来稳定后,可能不需要这些id,这只是中间状态。

Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@JZ-LIANG JZ-LIANG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JZ-LIANG JZ-LIANG merged commit 5620214 into PaddlePaddle:develop Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants