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 nll loss API #26019

Merged
merged 13 commits into from
Aug 12, 2020
Merged

add nll loss API #26019

merged 13 commits into from
Aug 12, 2020

Conversation

joey12300
Copy link
Contributor

PR types

New features

PR changes

APIs

Describe

Add nll_loss function, and encapsulate NLL_Loss class and optimize the performance of nll_loss in dygraph mode

@paddle-bot-old
Copy link

paddle-bot-old bot commented Aug 6, 2020

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

@joey12300 joey12300 force-pushed the add_nll_loss branch 3 times, most recently from 693432b to 60e7df1 Compare August 7, 2020 07:54
if x_dims != 2 and x_dims != 4:
x = paddle.reshape(x, shape=[n, c, 1, -1])
label = paddle.reshape(label, shape=[n, 1, -1])
out_shape = [n] + x_shape[2:]
Copy link
Contributor

Choose a reason for hiding this comment

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

这块是否要分一下静态图和动态图?

ignore_index, 'reduction',
reduction)
if x_dims != 2 and x_dims != 4 and reduction == 'none':
out = paddle.reshape(out, shape=out_shape)
Copy link
Contributor

Choose a reason for hiding this comment

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

用core.ops

@@ -505,18 +506,18 @@ class NLLLoss(fluid.dygraph.Layer):
\\end{cases}

Parameters:
input (Variable): Input tensor, the data type is float32, float64.
x (Variable): Input tensor, the data type is float32, float64.
Copy link
Contributor

Choose a reason for hiding this comment

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

Variable -> Tensor 下面都是把Variable改成Tensor

raise ValueError(
"The value of 'reduction' in nll_loss should be 'sum', 'mean' or "
"'none', but received %s, which is not allowed." % reduction)
x_shape = list(x.shape)
Copy link
Contributor

Choose a reason for hiding this comment

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

要确认一下Tensor这种shape属性的调用方式

@joey12300 joey12300 force-pushed the add_nll_loss branch 3 times, most recently from 826b71b to ee9ee56 Compare August 10, 2020 01:55
@joey12300 joey12300 force-pushed the add_nll_loss branch 4 times, most recently from f659ddf to 493292a Compare August 10, 2020 07:16
wawltor
wawltor previously approved these changes Aug 10, 2020

Parameters:
x (Tensor): Input tensor, the data type is float32, float64.
label (Tensor): Label tensor, the data type is int64_t.
Copy link
Contributor

Choose a reason for hiding this comment

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

Int64_t -> int64

Copy link
Contributor

Choose a reason for hiding this comment

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

按照规范copy一下

and does not contribute to the input gradient.
reduction (str, optional): Indicate how to average the loss,
the candicates are ``'none'`` | ``'mean'`` | ``'sum'``.
If :attr:`reduction` is ``'mean'``, the reduced mean loss is returned;
Copy link
Contributor

Choose a reason for hiding this comment

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

sum也要介绍一下

prog,
feed={"x": x_np,
"label": label_np},
fetch_list=[res])
Copy link
Contributor

Choose a reason for hiding this comment

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

静态图示例可以去掉

label = paddle.reshape(label, shape=[n, 1, -1])
out_shape = [n] + x_shape[2:]

helper = LayerHelper('nll_loss', **locals())
Copy link
Contributor

Choose a reason for hiding this comment

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

这块提前一下

reduction (str, optional): Indicate how to average the loss,
the candicates are ``'none'`` | ``'mean'`` | ``'sum'``.
If :attr:`reduction` is ``'mean'``, the reduced mean loss is returned;
Default is ``'mean'``.
ignore_index (int64, optional): Specifies a target value that is ignored
and does not contribute to the input gradient.
Copy link
Contributor

Choose a reason for hiding this comment

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

增加name参数


Returns:
The tensor variable storing the nll_loss.
The callable object which calculates negative log likelihood loss when
get the input `x` and `label` .
Copy link
Contributor

Choose a reason for hiding this comment

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

去掉x和label相关信息

@wawltor wawltor self-requested a review August 10, 2020 08:53
and does not contribute to the input gradient.
reduction (str, optional): Indicate how to average the loss,
the candicates are ``'none'`` | ``'mean'`` | ``'sum'``.
If :attr:`reduction` is ``'mean'``, the reduced mean loss is returned;
Copy link
Contributor

Choose a reason for hiding this comment

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

格式应该有问题 【If reduction is】就可以 下面同理

wawltor
wawltor previously approved these changes Aug 12, 2020
Copy link
Contributor

@wawltor wawltor left a comment

Choose a reason for hiding this comment

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

LGTM

yaoxuefeng6
yaoxuefeng6 previously approved these changes Aug 12, 2020
Copy link
Contributor

@yaoxuefeng6 yaoxuefeng6 left a comment

Choose a reason for hiding this comment

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

LGTM

"'none', but received %s, which is not allowed." % reduction)

if in_dygraph_mode():
x_shape = list(core.ops.shape(x))
Copy link
Contributor

Choose a reason for hiding this comment

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

you can write x_shape = x.shape directly here

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

"The value of 'reduction' in nll_loss should be 'sum', 'mean' or "
"'none', but received %s, which is not allowed." % reduction)
super(NLLLoss, self).__init__()
self.weight = weight
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe self._weight is better for private member, etc.

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

x (Tensor): Input tensor, the data type is float32, float64.
label (Tensor): Label tensor, the data type is int64.
weight (Tensor, optional): Weight tensor, a manual rescaling weight given
to each class. If given, it has to be a Tensor of size `C`. Otherwise,
Copy link
Contributor

Choose a reason for hiding this comment

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

it has to be a Tensor of size C

Better describe this clearly, does it means x should be of format [N, C, H, W]

Copy link
Contributor Author

@joey12300 joey12300 Aug 12, 2020

Choose a reason for hiding this comment

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

it has to be a Tensor of size C

Better describe this clearly, does it means x should be of format [N, C, H, W]

@zhiqiu Already describe the shape of x, label, weight more clearly.

@joey12300 joey12300 force-pushed the add_nll_loss branch 2 times, most recently from 0c5698e to 349c5e8 Compare August 12, 2020 08:42
@zhiqiu zhiqiu self-requested a review August 12, 2020 10:06
Copy link
Contributor

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

@wawltor wawltor left a comment

Choose a reason for hiding this comment

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

LGTM

@wawltor wawltor merged commit dea41da into PaddlePaddle:develop Aug 12, 2020
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

5 participants