-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Conversation
Thanks for your contribution! |
9505744
to
880078a
Compare
… auto_para_completion
@@ -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}; |
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.
如果明确该成员及相关构造函数仅用于自动并行的话,有没有可能通过继承实现?仅用于某一功能的成员放到基类中,设计上是不太好的,或者说此类成员在自动并行开发完善的过程中还是否会新增?还会新增多少?
这个可能要根据你们的实际情况来判断,如果确实必须加到全局基类中,我建议命名上直接带上功能信息,比如auto_parallel_id_
,明确这一成员的使用是有特定场景要求的,避免大家没看注释的时候困惑或者误用
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.
自动并行目前在python侧,采用非侵入式标记program方式,所以直接复用vardesc和opdesc,id和original_id应该是仅有需求,如果还增加新的,的确要考虑继承或者分拆类,另外未来稳定后,可能不需要这些id,这只是中间状态。
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.
LGTM
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.
LGTM
PR types
Others
PR changes
Others
Describe
id
of every var desc and op desc be unique.original_id
to record the serial desc which the distributed one copies from.GenerateId
toprivate
.