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

[Dy2St] Move more transformer specific utilities to transformers dir #61242

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Jan 27, 2024

PR types

Others

PR changes

Others

Description

#61193,将剩余的仅转写相关的 utilities 移动到 transformers/utils

至此,utilstransformers/utils 都控制在了可接受的 600 行左右,且各自职责清晰

  • utils 处于底层,任何位置皆可引用
  • transformers/utils 仅用于 transformers 目录,不应该在其他位置引用(除了单测),如果想要用于其它模块,需要放置在 utils 而不是 transformers/utilsutils 保留的少数针对于 AST Node 的函数就是由于此原因

由于短期内动转静不会有较大的迭代,utils 控制在 600 行左右应该不会继续膨胀(老 IR 退场后甚至能清更多代码),如果后续迭代超过 1000 行,则应继续考虑拆分 utils 了

PCard-66972

@SigureMo SigureMo requested a review from gouzil January 27, 2024 19:11
Copy link
Member

@gouzil gouzil left a comment

Choose a reason for hiding this comment

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

LGTM

@SigureMo SigureMo merged commit b3308ea into PaddlePaddle:develop Jan 29, 2024
29 of 30 checks passed
@SigureMo SigureMo deleted the dy2st/move-more-transformer-specific-utilities-to-transformers-dir branch January 29, 2024 03:12
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.

None yet

3 participants