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

Add lrn layer #9157

Merged
merged 10 commits into from
Mar 26, 2018
Merged

Add lrn layer #9157

merged 10 commits into from
Mar 26, 2018

Conversation

dragonwarrior
Copy link
Contributor

@dragonwarrior dragonwarrior commented Mar 16, 2018

Now the LRN is not exported to Python, I add a LRN layer in nn.py, please have a look.

@CLAassistant
Copy link

CLAassistant commented Mar 16, 2018

CLA assistant check
All committers have signed the CLA.

@qingqing01 qingqing01 self-requested a review March 16, 2018 07:05
@qingqing01
Copy link
Contributor

@luotao1
Copy link
Contributor

luotao1 commented Mar 16, 2018

As a supplement, doc format standard is in
https://github.com/PaddlePaddle/Paddle/blob/develop/doc/fluid/dev/api_doc_std_cn.md
@ranqiu92 can you help review the doc format of LRN as well?

k(float): An offset (usually positive to avoid dividing by 0)
alpha(float): The scaling parameter
beta(float): The exponent
name(str): A name for this operation
Copy link
Contributor

Choose a reason for hiding this comment

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

功能介绍部分请简单介绍下LRN,给出参考文献及链接(如果有)。
参数介绍部分,遗漏了input,没有写明默认值,缺少必要的标点。
还存在其他问题。请参考 API注释撰写标准 撰写注释。同时注意语言上的问题。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@pkuyym
Copy link
Contributor

pkuyym commented Mar 18, 2018

Please add a test for your wrapper.

**Local Response Normalization Operator**

This operator comes from the paper:
<<ImageNet Classification with Deep Convolutional Neural Networks>>.
Copy link
Contributor

Choose a reason for hiding this comment

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

参考文献 请移动至 参数介绍前,另去掉书名号,加上文献链接。
此处请添加 功能介绍。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

1.参考文献 请移动至 公式符号介绍后,参数介绍前。
2. 补充功能介绍

k(float, default 2.0): An offset (usually positive to avoid dividing by 0).
alpha(float, default 1e-4): The scaling parameter.
beta(float, default 0.75): The exponent.
name(str, default None): A name for this operation.
Copy link
Contributor

Choose a reason for hiding this comment

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

L105 不要使用 简写 如 dims
L105~L110 参数名和括号间请加一个空格。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


data = fluid.layers.data(name="data", shape=[3, 112, 112], dtype="float32")
lrn = fluid.layers.lrn(input=data)
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete this file.

qingqing01
qingqing01 previously approved these changes Mar 19, 2018
Copy link
Contributor

@qingqing01 qingqing01 left a comment

Choose a reason for hiding this comment

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

LGTM. @ranqiu92 please help to look again. Thanks!

"""
**Local Response Normalization Operator**

Refer to `ImageNet Classification with Deep Convolutional Neural Networks
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. 请补充api的功能介绍。
  2. 参考文献 放置 参数介绍前 公式符号介绍后。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Args:
input (Variable): The input tensor of this layer, and the dimension of input tensor must be 4.
n (int, default 5): The number of channels to sum over.
k (float, default 1.0): An offset (usually positive to avoid dividing by 0).
Copy link
Contributor

Choose a reason for hiding this comment

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

avoid being divided by 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.

done

@ranqiu92
Copy link
Contributor

@luotao1 注释的格式差不多了,你看下内容、表达上怎么样?

@luotao1
Copy link
Contributor

luotao1 commented Mar 20, 2018

@ranqiu92 内容上由 @qingqing01 把关即可。
And for the presentation of lrn doc, @abhinavarora could you help review it?

qingqing01
qingqing01 previously approved these changes Mar 20, 2018
Copy link
Contributor

@qingqing01 qingqing01 left a comment

Choose a reason for hiding this comment

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

LGTM.

@abhinavarora
Copy link
Contributor

@luotao1 Sure, I will review it ASAP.

Copy link
Contributor

@abhinavarora abhinavarora left a comment

Choose a reason for hiding this comment

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

@luotao1 Most of the documentation is good. There is a small suggestion. Please fix that before merging. Thank you for the great PR!


def lrn(input, n=5, k=1.0, alpha=1e-4, beta=0.75, name=None):
"""
Local Response Normalization Layer. This layer performs a kind of
Copy link
Contributor

Choose a reason for hiding this comment

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

kind -> type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@abhinavarora abhinavarora left a comment

Choose a reason for hiding this comment

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

LGTM!

@qingqing01 qingqing01 merged commit 54a85b7 into PaddlePaddle:develop Mar 26, 2018
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

8 participants