Skip to content

Conversation

@phlrain
Copy link
Collaborator

@phlrain phlrain commented Feb 13, 2022

PR types

Breaking changes

PR changes

OPs

Describe

move histomgram op to pten

@paddle-bot-old
Copy link

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

kernel_ctx->EmplaceBackAttr(BOOST_GET_CONST(float, attr));
} else if (attr_defs[i].type_index == std::type_index(typeid(bool))) {
kernel_ctx->EmplaceBackAttr(BOOST_GET_CONST(bool, attr));
} else if (attr_defs[i].type_index == std::type_index(typeid(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.

缩进多了一个空格?

Comment on lines 88 to 92
// REGISTER_OP_CPU_KERNEL(
// histogram, ops::HistogramKernel<paddle::platform::CPUDeviceContext, float>,
// ops::HistogramKernel<paddle::platform::CPUDeviceContext, double>,
// ops::HistogramKernel<paddle::platform::CPUDeviceContext, int>,
// ops::HistogramKernel<paddle::platform::CPUDeviceContext, 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.

这段注释要不要去掉?

Comment on lines 13 to 18
const DenseTensor& input,
int64_t bins,
int min,
int max,
DenseTensor* output)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

缩进好像有点问题?

@@ -0,0 +1,77 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

License?

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

Copy link
Contributor

Choose a reason for hiding this comment

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

License?

@@ -0,0 +1,16 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

License?

Comment on lines 10 to 14
const DenseTensor& input,
int64_t bins,
int min,
int max,
DenseTensor* out);
Copy link
Contributor

Choose a reason for hiding this comment

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

同缩进似乎有问题?

@@ -0,0 +1,15 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

License?

Comment on lines 89 to 91
input_min_t.mutable_data<T>({1}, dev_ctx.GetPlace());
auto* input_max_data =
input_max_t.mutable_data<T>({1}, context.GetPlace());
auto input_min_scala = framework::EigenScalar<T>::From(input_min_t);
auto input_max_scala = framework::EigenScalar<T>::From(input_max_t);
input_max_t.mutable_data<T>({1}, dev_ctx.GetPlace());
Copy link
Contributor

Choose a reason for hiding this comment

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

这里要替换成Alloc吗?

const T* input_data = input.data<T>();
auto input_numel = input.numel();

int64_t* out_data = output->mutable_data<int64_t>(dev_ctx.GetPlace());
Copy link
Contributor

Choose a reason for hiding this comment

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

这里要替换成Alloc吗?

float,
double,
int,
int64_t) {} No newline at end of file
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

@MingMingShangTian MingMingShangTian left a comment

Choose a reason for hiding this comment

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

LGTM

@phlrain phlrain merged commit 556f6eb into develop Feb 15, 2022
@luotao1 luotao1 deleted the move_histogram_to_pten branch February 22, 2022 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants