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

remove nets.py in fluid #51717

Merged
merged 35 commits into from
Jun 28, 2023
Merged

remove nets.py in fluid #51717

merged 35 commits into from
Jun 28, 2023

Conversation

longranger2
Copy link
Contributor

@longranger2 longranger2 commented Mar 15, 2023

PR types

Others

PR changes

APIs

Description

To clean the code in paddle.fluid, the useless codes should be removed and code still be used should move to a new position.

fluid 目录下的 nets.py 包含较多预定义的简单网络,已基本在1期工作中改成了2.0API组网。只在单测使用,因此将其迁移到单测里提供单测的组网依赖。

@paddle-bot
Copy link

paddle-bot bot commented Mar 15, 2023

你的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.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Mar 15, 2023

__all__ = [
"simple_img_conv_pool",
"sequence_conv_pool",
"glu",
"scaled_dot_product_attention",
"img_conv_group",
]
Copy link
Contributor

Choose a reason for hiding this comment

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

__all__没必要保留了

out = paddle.multiply(x=a, y=act_b)
return out


def scaled_dot_product_attention(
Copy link
Contributor

Choose a reason for hiding this comment

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

需要明确每条单测的作用,进而确定单测使用到的函数的作用,原则上我们不为某个单测保留函数:

  1. 如果某个单测核心目的是测试某个其他功能,只是借助到这里的某个函数组网,那么这个函数可以作为依赖保留。例如simple_img_conv_pool
  2. 如果某个单测的核心目的就是测试这个函数,那么如果函数可以移除的话,单测也相应移除即可。例如scaled_dot_product_attention

@@ -41,7 +41,7 @@ def setUp(self):
def check_identity(self, place):
with dg.guard(place):
x_var = dg.to_variable(self.x)
y_var = fluid.nets.glu(x_var, self.dim)
y_var = paddle.nn.functional.glu(x_var, self.dim)
Copy link
Contributor

Choose a reason for hiding this comment

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

下面已有一个测试nn.functional.glu 的单测TestGLUV2,这个单测可直接移除

@@ -23,7 +23,7 @@
import paddle
import paddle.fluid as fluid
import paddle.fluid.layers as layers
import paddle.fluid.nets as nets
import paddle.fluid.tests.unittests.nets as nets
Copy link
Contributor

Choose a reason for hiding this comment

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

这里能否直接用.nets,这样使用会有类似API的感觉,下同。

@longranger2
Copy link
Contributor Author

修改好了~

use_double_buffer=False,
)

conv_pool_1 = fluid.nets.simple_img_conv_pool(
Copy link
Contributor

Choose a reason for hiding this comment

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

这个单测,从文件名看是测试执行器的,不应当在这个PR里移除

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我刚提交PR的时候发生冲突,发现这个文件test_async_ssa_graph_executor_mnist.py已经整个被删除了,所以才把这个文件删了来解决冲突的

@@ -44,7 +46,7 @@ def set_program(self):
)
keys.stop_gradient = False

contexts = fluid.nets.scaled_dot_product_attention(
contexts = nets.scaled_dot_product_attention(
Copy link
Contributor

Choose a reason for hiding this comment

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

这个函数已经在上面移除了,会报错。另外这个单测中,本质是测试这一个api的网络,看起来这个单测可以整体移除掉。

zoooo0820
zoooo0820 previously approved these changes Jun 6, 2023
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

XieYunshen
XieYunshen previously approved these changes Jun 6, 2023
Copy link
Contributor

@XieYunshen XieYunshen left a comment

Choose a reason for hiding this comment

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

LGTM
单测删除

@longranger2 longranger2 dismissed stale reviews from XieYunshen and zoooo0820 via 193cc19 June 6, 2023 13:35
zhwesky2010
zhwesky2010 previously approved these changes Jun 7, 2023
Copy link
Contributor

@zhwesky2010 zhwesky2010 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 rm UT

Copy link
Contributor

@zhwesky2010 zhwesky2010 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 rm UT

@longranger2
Copy link
Contributor Author

longranger2 commented Jun 8, 2023

book目录下的cmake和contrib目录下的cmake 定义的函数py_test对Linux/win效果不一样。

image

image

分布式文件因为在win下不跑所以没这个问题

image

XieYunshen
XieYunshen previously approved these changes Jun 26, 2023
Copy link
Contributor

@jeff41404 jeff41404 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

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

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

no doc's changes. LGTM

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

@jeff41404 jeff41404 merged commit 87f7210 into PaddlePaddle:develop Jun 28, 2023
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants