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

【PaddlePaddle Hackathon 2】23、为 Paddle 新增 Softmax2D 组网API #40910

Merged
merged 11 commits into from Apr 21, 2022
Merged

【PaddlePaddle Hackathon 2】23、为 Paddle 新增 Softmax2D 组网API #40910

merged 11 commits into from Apr 21, 2022

Conversation

Asthestarsfalll
Copy link
Contributor

@Asthestarsfalll Asthestarsfalll commented Mar 24, 2022

PR types

Others

PR changes

APIs

Describe

解决了issue: #40310
增加了paddle.nn.Softmax2D。具体地,paddle.nn.Softmax2D()在空间维度计算softmax,从而使输出 tensor 在每个空间维度(channels,hi,wj)的 tensor 求和为1,即表示空间位置上的某个通道向量求和为1。
设计文档:PaddlePaddle/community#50
中文文档:PaddlePaddle/docs#4543

@paddle-bot-old paddle-bot-old bot added contributor External developers status: proposed labels Mar 24, 2022
@paddle-bot-old
Copy link

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

@dingjiaweiww
Copy link
Contributor

请先通过CI噢~

@dingjiaweiww
Copy link
Contributor

PR-CI-Windows这个CI是需要通过的哦~

@Asthestarsfalll
Copy link
Contributor Author

@dingjiaweiww 你好,已通过

@shiyutang
Copy link
Contributor

请注意代码PR中还需要和中文文档关联才可合入,可以参考这个PR:#40472
image

书写规范请参考:https://github.com/PaddlePaddle/docs/wiki/%E9%A3%9E%E6%A1%A8API%E6%96%87%E6%A1%A3%E4%B9%A6%E5%86%99%E8%A7%84%E8%8C%83

Copy link
Contributor

@shiyutang shiyutang left a comment

Choose a reason for hiding this comment

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

代码整体完整,部分细节需要完善

@@ -0,0 +1,90 @@
# Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

空格对齐

# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import print_function
Copy link
Contributor

Choose a reason for hiding this comment

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

这个不需要

from test_softmax_op import ref_softmax

paddle.enable_static()
np.random.seed(2022)
Copy link
Contributor

Choose a reason for hiding this comment

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

不需要固定随机种子

import paddle.fluid.core as core
from test_softmax_op import ref_softmax

paddle.enable_static()
Copy link
Contributor

Choose a reason for hiding this comment

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

这个可以放到 def test_static_api(self) 中

else paddle.CPUPlace()

def test_extra_repr(self):
paddle.disable_static(self.place)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里相应修改

self.x_np = np.random.uniform(-1, 1, self.shape).astype('float64')
self.axis = -3
self.place = paddle.CPUPlace()

Copy link
Contributor

Choose a reason for hiding this comment

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

增加GPU上单独测试、shape不是3或4维度的错误测试(可以使用assertRaises(error, func,input))



class TestSoftmax2DAPI(unittest.TestCase):
# test paddle.nn.Softmax2D
Copy link
Contributor

Choose a reason for hiding this comment

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

不必要的注释可以删去

# test paddle.nn.Softmax2D
def setUp(self):
self.shape = [2, 6, 5, 4]
self.dtype = 'float64'
Copy link
Contributor

Choose a reason for hiding this comment

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

输入默认需要支持float32,可以增加一个float32的测试

@Asthestarsfalll Asthestarsfalll changed the title Hackathon 23 【PaddlePaddle Hackathon 2】23、为 Paddle 新增 Softmax2D 组网API Apr 8, 2022
@Asthestarsfalll
Copy link
Contributor Author

已修改并添加中文文档

@dingjiaweiww
Copy link
Contributor

PR-CI-Kunlun和PR-CI-Windows-Inference这两个CI需要通过的噢~

@Asthestarsfalll
Copy link
Contributor Author

@dingjiaweiww 你好,已通过

Copy link
Contributor

@shiyutang shiyutang left a comment

Choose a reason for hiding this comment

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

LGTM

@shiyutang shiyutang requested a review from DDDivano April 12, 2022 03:34
x_np = np.random.randn(2, 3, 4, 2, 3)
x = paddle.to_tensor(x_np, dtype='float64')
m = paddle.nn.Softmax2D()
self.assertRaises(AssertionError, m, x)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里为什么是个AssertionError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

因为代码中使用了assert

assert x.ndim == 3 or x.ndim == 4

Copy link
Contributor

Choose a reason for hiding this comment

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

开发者在python层面实现避免输入维度不符合期望使用了assert,因此维度不匹配会触发AssertionError。

def setUp(self):
self.shape = [2, 3, 4]
self.x_np = np.random.uniform(-1, 1, self.shape).astype('float32')
self.axis = -3
Copy link
Contributor

Choose a reason for hiding this comment

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

axis可以覆盖不同的值,不需要都是-3,比如0, 正数

Copy link
Contributor Author

Choose a reason for hiding this comment

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

对于softmax2d来说,需要将softmax的axis固定为-3,这里只是显式的写出来了,实际上不需要变动

Copy link
Contributor

Choose a reason for hiding this comment

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

这里主要因为 softmax2D 的功能需求是固定了axis=-3的,如果不设置成-3,numpy的实现ref_softmax的就不是实现softmax2D的功能了。那么不能用于和softmax2D的对比测试了。

@paddle-bot-old
Copy link

PR格式检查通过,你的PR将接受Paddle专家以及开源社区的review,请及时关注PR动态。
The format inspection passed. Your PR will be reviewed by experts of Paddle and developers from the open-source community. Stay tuned.

@paddle-bot-old
Copy link

Sorry to inform you that d0484ba's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

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.

LG API

@jeff41404 jeff41404 merged commit 920d44d into PaddlePaddle:develop Apr 21, 2022
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