Skip to content

Conversation

@linjieccc
Copy link
Contributor

@linjieccc linjieccc commented Mar 4, 2022

PR types

Function optimization

PR changes

OPs

Describe

move viterbi_decode to phi

@paddle-bot-old
Copy link

paddle-bot-old bot commented Mar 4, 2022

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

out->set_dtype(x.dtype());
}

void ViterbiDecodeInferMeta(const MetaTensor& input,
Copy link
Contributor

Choose a reason for hiding this comment

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

这里有三个tensor参数?是不是应该属于ternary.h/cc

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

namespace funcs {

template <typename Context, typename T>
inline void TransCompute(const int dim,
Copy link
Contributor

Choose a reason for hiding this comment

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

这个有没有可能直接用transpose_kernel.h中的Transpose替代,避免重复封装

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 TensorBuffer {
Copy link
Contributor

Choose a reason for hiding this comment

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

这种在内部封装类似Tensor的结构是应该避免的,我们后续优化下这个算子

#include <string>
#include <vector>

#include "paddle/fluid/operators/elementwise/elementwise_op_function.h"
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.

这里LaunchElementwiseCudaKernel需要用到

Comment on lines 17 to 19
#ifdef PADDLE_WITH_MKLML
#include <omp.h>
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

我看到funcs/viterbi_decode_functor.h下也用到了这个头文件的内容,这个头文件引用是否放在那个文件里更合适一点?

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

Comment on lines 172 to 174
DenseTensor int_buffer;
int_buffer.Resize(phi::make_ddim({buffer_size}));
dev_ctx.template Alloc<int64_t>(&int_buffer);
Copy link
Contributor

Choose a reason for hiding this comment

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

这种写法可以试试使用 int_buffer = Empty<int64_t>(dev_ctx, {buffer_size});, 写起来会简单一些

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

Comment on lines 180 to 182
DenseTensor float_buffer;
float_buffer.Resize(phi::make_ddim({buffer_size}));
dev_ctx.template Alloc<T>(&float_buffer);
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, thx

Copy link
Contributor

@chenwhql chenwhql left a comment

Choose a reason for hiding this comment

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

LGTM

@chenwhql chenwhql merged commit b97e6d1 into PaddlePaddle:develop Mar 9, 2022
@linjieccc linjieccc deleted the move_viterbi branch March 9, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants