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 some comment of CrossMapNormalFunc #1202

Merged
merged 4 commits into from
Jan 23, 2017

Conversation

hedaoyuan
Copy link
Contributor

@hedaoyuan hedaoyuan commented Jan 20, 2017

According to the comment of @emailweixu .
Each Function needs a comment, and

  1. Do not need to read the code, to know what Function is doing
  2. Do not need to read the code, to know the meaning of the Function parameter, and how to use this Function;

Normalization is a complex formula, I am not sure if it's clear. Please help to review this pr.

* alpha /min(F, f-[N/2] + N)
* (1 + ----- * | (Input(x, y))^2 ) ^ (beta)
* N /max(0, f-[N/2])
*
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. 看这个公式,无法看出 across channel (or maps)
  2. Input/Output是4D的tensor,注释里面如果忽略第一维的话, Input(x, y) 是不是应该写作 Input(c, x, y) (或者 Input[, , , ]) ?
  3. 求和符号, 直接用 sum 会不会更清晰些, 或者
   --
   \
   /
   --
  1. paper的公式里是 +k,而不是+1。
  2. alpha不用除N吧?

另外,如果比较复杂的公式,如果拆解开来写多行,会不会好些一些

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.

*
* Argument in the Function:
* \param size_ represent N
* \param scale_ represent alpha / N
Copy link
Contributor

Choose a reason for hiding this comment

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

paper的公式,直接是 alpha,看代码实现了scale_等价公式里的alpha吧? 不是alpha/N吧? paper: https://papers.nips.cc/paper/4824-imagenet-classification-with-deep-convolutional-neural-networks.pdf

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

@tianbingsz tianbingsz left a comment

Choose a reason for hiding this comment

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

LGTM.

@hedaoyuan hedaoyuan merged commit 39f6972 into PaddlePaddle:develop Jan 23, 2017
lizexu123 pushed a commit to lizexu123/Paddle that referenced this pull request Feb 23, 2024
* Update Some Demos & Add Infer for ImageNet Demo

* Update Some Demos & Add Infer for ImageNet Demo

* Update Some Demos & Add Infer for ImageNet Demo

* Update Some Demos & Add Infer for ImageNet Demo
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.

3 participants