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

[bug fix]remove paddle.nn.Sequential to fix dygraph to static error #48477

Conversation

GGBond8488
Copy link
Contributor

@GGBond8488 GGBond8488 commented Nov 28, 2022

PR types

Bug fixes

PR changes

APIs

Describe

In the previous work of cleaning Sequential under fluid, due to the problem of circular dependency, I copied a copy of Sequential and put it under paddle.nn, and kept the original Sequential under fluid.

In dygraph to static, paddle.fluid.dygraph.dygraph_to_static.convert_call_func.py imports paddle.fluid.dygraph.container.Sequential under fluid on line 27, and performs type judgment on lines 48 and 131, which will lead to an obscure problem. Use paddle.nn.Sequential models will not be able to perform dygraph and static conversions correctly.

So we remove the Sequential under paddle.nn, and will migrate it back after the dygraph to static migration is completed.

@paddle-bot
Copy link

paddle-bot bot commented Nov 28, 2022

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

Copy link
Contributor

@zoooo0820 zoooo0820 left a comment

Choose a reason for hiding this comment

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

LGTM

@GGBond8488
Copy link
Contributor Author

GGBond8488 commented Nov 29, 2022

之前因为循环依赖的问题,将Sequential复制一份实现放到paddle.nn下,保留原有Sequential实现
在paddle.fluid.dygraph.dygraph_to_static.convert_call_func.py引入了fluid下的Sequential
line 27from paddle.fluid.dygraph.container import Sequential(由于循环依赖无法替换)
line 48 PADDLE_NEED_CONVERT_APIS = [Sequential]
line 131 if type(func) in PADDLE_NEED_CONVERT_APIS: return False
会导致使用paddle.nn.Sequential的部分模型动转静失败(两个Sequential不是同一个类)

等到动转静完成迁出fluid以后,可将Sequential 从fluid下完全迁移到paddle.nn

@jeff41404
Copy link
Contributor

The description should be clear why remove paddle.nn Sequential? and what conditions can be met to migrate Sequential in fluid to paddle.nn.Sequential ?

@GGBond8488
Copy link
Contributor Author

The description should be clear why remove paddle.nn Sequential? and what conditions can be met to migrate Sequential in fluid to paddle.nn.Sequential ?

added description

Copy link
Contributor

@Ligoml Ligoml left a comment

Choose a reason for hiding this comment

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

LGTM for docs

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