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

Update initializer examples of Bilinear #27709

Merged
merged 5 commits into from
Oct 9, 2020
Merged

Conversation

Ray2020BD
Copy link
Contributor

PR types

Function optimization

PR changes

APIs

Describe

Update initializer examples of Bilinear

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

regularizer=L2Decay(0.),
initializer=nn.initializer.Bilinear())
data = np.random.uniform(-1, 1, [30, 10, 32]).astype('float32')
data = paddle.to_tensor(data)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里直接用paddle.rand吧。
上面也不需要import numpy和disable_static了。
另外上面的where, ... 那部分描述还要留着呀。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,已经完成了

Copy link
Member

Choose a reason for hiding this comment

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

paddle.rand返回已经是个tensor,所以不用data = paddle.to_tensor(data),请删除745行

@Ray2020BD Ray2020BD changed the title test=document_fix Update initializer examples of Bilinear Sep 29, 2020
@@ -729,31 +729,32 @@ class BilinearInitializer(Initializer):

.. code-block:: python

import paddle.fluid as fluid
import paddle
Copy link
Member

Choose a reason for hiding this comment

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

Suggest to change import order. Usually we group the imports by function, then sort it by alphabetical order. Here import math is in the middle of import paddle, which is ugly.

Suggest to change like this:

import math

import paddle
import paddle.nn
from paddle.regularizer import L2Decay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've already modified.

Copy link
Member

@zhhsplendid zhhsplendid Sep 29, 2020

Choose a reason for hiding this comment

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

Could you just put in order? You changed it like:

import xxx
from xxx import
import xxx

Why can't you just make it pretty? Since all xxx are paddle

import xxx
import xxx
from xxx import

regularizer=L2Decay(0.),
initializer=nn.initializer.Bilinear())
data = np.random.uniform(-1, 1, [30, 10, 32]).astype('float32')
data = paddle.to_tensor(data)
Copy link
Member

Choose a reason for hiding this comment

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

paddle.rand返回已经是个tensor,所以不用data = paddle.to_tensor(data),请删除745行

regularizer=L2Decay(0.),
initializer=nn.initializer.Bilinear())
data = paddle.rand([B, 3, H, W], dtype='float32')
#data = paddle.to_tensor(data)
Copy link
Member

Choose a reason for hiding this comment

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

Delete, not comment. Why comment? This example code is used for user reading, if you put comment here, it may confuse user!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already delete.

@@ -729,31 +729,32 @@ class BilinearInitializer(Initializer):

.. code-block:: python

import paddle.fluid as fluid
import paddle
Copy link
Member

@zhhsplendid zhhsplendid Sep 29, 2020

Choose a reason for hiding this comment

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

Could you just put in order? You changed it like:

import xxx
from xxx import
import xxx

Why can't you just make it pretty? Since all xxx are paddle

import xxx
import xxx
from xxx import

Copy link
Member

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

@jzhang533 jzhang533 left a comment

Choose a reason for hiding this comment

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

lgtm

@liym27 liym27 merged commit 6da552a into PaddlePaddle:develop Oct 9, 2020
@Ray2020BD Ray2020BD deleted the rc20 branch October 9, 2020 08:33
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