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

Unify the metrics implementation between low-level and high-level API. #26158

Merged
merged 21 commits into from
Aug 24, 2020

Conversation

qingqing01
Copy link
Contributor

@qingqing01 qingqing01 commented Aug 11, 2020

PR types

Function optimization

PR changes

APIs

Describe

  1. Unify the metrics implementation between low-level and high-level API.
  2. Change function name add_metric_op in Metric to compute.
  3. Return scalar, not [scalar] in update() and accumulate() of Accuracy.
  4. Refine and add comments for sub-function in Accuracy.
    • Add comments for compute, update, reset..
    • Add standalone usage and usage with Model API for Metrics
  5. Add unit testing in python/paddle/tests

@paddle-bot-old
Copy link

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

@paddle-bot-old
Copy link

paddle-bot-old bot commented Aug 11, 2020

✅ This PR's description meets the template requirements!
Please wait for other CI results.

LielinJiang
LielinJiang previously approved these changes Aug 17, 2020
| ||
outputs & labels ||
| || tensor data
{Metric.compute} ||
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

test=develop
heavengate
heavengate previously approved these changes Aug 17, 2020
"""
Configures the model before runing.

Args:
optimizer (Optimizer|None): Optimizer must be set in training
and should be a Optimizer instance. It can be None in eval
and test mode.
loss_function (Loss|callable function|None): Loss function can
loss (Loss|callable function|None): Loss function can
be a `fluid.dygraph.Layer` instance or any callable function

Choose a reason for hiding this comment

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

fluid.dygraph.Layer => paddle.nn.Layer ,下面用到fluid.dygraph.Layer的地方同样都需要修改一下

@@ -1362,15 +1362,15 @@ def evaluate(
input = hapi.Input('image', [-1, 1, 28, 28], 'float32')
label = hapi.Input('label', [None, 1], 'int64')
model = hapi.Model(hapi.vision.LeNet(), input, label)
model.prepare(metrics=hapi.metrics.Accuracy())
model.prepare(metrics=paddle.metric.Accuracy())

result = model.evaluate(val_dataset, batch_size=64)
print(result)

# imperative mode
fluid.enable_dygraph()

Choose a reason for hiding this comment

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

fluid.enable_dygraph => paddle.disable_static

guoshengCS
guoshengCS previously approved these changes Aug 17, 2020

Args:
name (str, optional): String name of the metric instance.
Default is `precision`.
Copy link
Contributor

Choose a reason for hiding this comment

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

这里name默认值是否应是recall

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:
name (str, optional): String name of the metric instance. Default
is `acc`.
Copy link
Contributor

Choose a reason for hiding this comment

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

这里name默认值是否应是 auc

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

name (str, optional): String name of the metric instance. Default
is `acc`.
curve (str): Specifies the mode of the curve to be computed,
'ROC' or 'PR' for the Precision-Recall-curve. Default is 'ROC'.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add doc for num_thresholds later

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.

.. code-block:: python
def compute(pred, label):
# sort prediction and slice the top-5 scores
pred = fluid.layers.argsort(pred, descending=True)[1][:, :5]

Choose a reason for hiding this comment

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

argsort用paddle.下面的API,去掉fluid

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. Thx!

pred = fluid.layers.argsort(pred, descending=True)[1][:, :5]
# calculate whether the predictions are correct
correct = pred == label
return fluid.layers.cast(correct, dtype='float32')

Choose a reason for hiding this comment

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

去掉fluid

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. Thx!

class Accuracy(Metric):
"""
Encapsulates accuracy metric logic.
def __init__(self, topk=(1, ), name='acc', *args, **kwargs):

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.

Removed. Thanks!

import paddle

paddle.disable_static()
x = paddle.to_variable(np.array([

Choose a reason for hiding this comment

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

to_variable => to_tensor

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

[0.1, 0.4, 0.3, 0.2],
[0.1, 0.2, 0.4, 0.3],
[0.1, 0.2, 0.3, 0.4]]))
y = paddle.to_variable(np.array([[0], [1], [2], [3]]))

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

.. code-block:: python

import paddle
import paddle.fluid as fluid

Choose a reason for hiding this comment

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

去掉fluid

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

train_dataset = hapi.datasets.MNIST(mode='train')

model = hapi.Model(hapi.vision.LeNet(classifier_activation=None))
optim = fluid.optimizer.Adam(

Choose a reason for hiding this comment

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

paddle.optimizer.Adam

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

@qingqing01 qingqing01 dismissed stale reviews from guoshengCS and heavengate via 362d914 August 18, 2020 04:22



from ..fluid.metrics import Accuracy, Auc, ChunkEvaluator, CompositeMetric, DetectionMAP, EditDistance, \
Copy link
Contributor

Choose a reason for hiding this comment

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

paddle/fluid/metrics.py 里的API计划怎么处理?
还有ChunkEvaluator, CompositeMetric, DetectionMAP, EditDistance在重构后的paddle.metric下没有对应的API。

Copy link
Contributor

Choose a reason for hiding this comment

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

@XiaoguangHu01 晓光麻烦看看__init__.py文件的改动。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

相关被使用统计在agroup中。 DetectionMAP、EditDistance建议先废弃,相关套件已在模型里重新实现。Precision/Recall当前没有被任何模型使用到,当前fluid下面的实现还太简单。 会上讨论,本次PR先以统一位置为主,后续版本还需要增强相关功能,包括增加ChunkEvaluator,以及改进功能。

Copy link
Contributor

Choose a reason for hiding this comment

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

计划废弃的API都添加一下deprecated decorator吧。

from ..fluid.metrics import Accuracy, Auc, ChunkEvaluator, CompositeMetric, DetectionMAP, EditDistance, \
Precision, Recall
from .metrics import *
from . import metrics

from ..fluid.layers.metric_op import accuracy, auc
from ..fluid.layers.nn import chunk_eval, cos_sim, mean_iou
Copy link
Contributor

Choose a reason for hiding this comment

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

paddle.metric下的class形式API,跟小写的调用op的API的关系,是否讨论过啊?

Copy link
Contributor Author

@qingqing01 qingqing01 Aug 20, 2020

Choose a reason for hiding this comment

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

class形式: 提供batch累积功能。一般用于高层API。底层API也可以使用。 参考内部wiki记录。

function(op形式): 保留了fluid下面的api,一般用于底层API。

  • Accuracy: 只支持当前batch的统计,需用户做batch累积计算。底层API可以使用。
  • auc: 可计算batch和累积batch的auc。 PaddleRec中使用。

model.prepare(
optim,
loss=nn.BCELoss(),
metrics=paddle.metric.Recall())
Copy link
Contributor

Choose a reason for hiding this comment

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

这里给一个可以同时计算precison和recall的例子?
通常情况下会同时算这两个指标的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jzhang533 Precision和Recall的功能会上讨论后续的版本还需增强,见上面回复,本次先以统一为主。所以后续可以增强下。

Copy link
Contributor

Choose a reason for hiding this comment

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

我看model.prepare的metrics是可以接受list的,这里改一下是否就可以?(先不考虑运行效率问题)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jzhang533 Done. Thanks!

"Auc.update",
"Auc.accumulate",
"Auc.name",
"Auc.compute",
Copy link
Contributor

Choose a reason for hiding this comment

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

同意这次对于白名单的改动。因为在class Auc等的地方已经有了示例代码。

XiaoguangHu01
XiaoguangHu01 previously approved these changes Aug 21, 2020
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.

LTGM

jzhang533
jzhang533 previously approved these changes Aug 22, 2020
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
will have follow up pr to deprecate fluid apis.

raindrops2sea
raindrops2sea previously approved these changes Aug 23, 2020
saxon-zh
saxon-zh previously approved these changes Aug 23, 2020
Copy link

@saxon-zh saxon-zh 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
Copy link
Contributor Author

Fix sample code about Adam, since #26288 is merged.

@XiaoguangHu01 @raindrops2sea @jzhang533 @saxon-zh Please help to review again.

Copy link

@saxon-zh saxon-zh 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

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

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

@qingqing01 qingqing01 merged commit 78ca8cf into PaddlePaddle:develop Aug 24, 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.

8 participants