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

support vector<float64> as type of op attribute and op set_value suppport float64 numpy.array #30126

Merged
merged 1 commit into from
Jan 11, 2021

Conversation

liym27
Copy link
Contributor

@liym27 liym27 commented Jan 5, 2021

PR types

New features

PR changes

OPs

Describe

  1. Support vector<float64> as type of op attribute.
  2. op set_value suppports float64 numpy.array

  • TODO: 处理精度问题。属性为 vector 时,c++端处理时会转为 vector,精度损失。

@paddle-bot-old
Copy link

paddle-bot-old bot commented Jan 5, 2021

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 Jan 5, 2021

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

@liym27 liym27 force-pushed the setitem_support_attr_float64 branch from eb908ce to f78a3e9 Compare January 5, 2021 11:30
@liym27 liym27 force-pushed the setitem_support_attr_float64 branch from f78a3e9 to b114d7a Compare January 5, 2021 12:06
@liym27 liym27 changed the title support float64 support vector<float64> as type of op attribute and op set_value suppport float64 numpy.array Jan 6, 2021
@liym27 liym27 force-pushed the setitem_support_attr_float64 branch from b114d7a to 52b40d4 Compare January 6, 2021 03:23
std::vector<double>* operator()(Attribute& attr) const {
if (attr.type() == typeid(std::vector<int>)) { // NOLINT
std::vector<int> val = BOOST_GET_CONST(std::vector<int>, attr);
std::vector<double> vec(val.begin(), val.end());
Copy link
Member

Choose a reason for hiding this comment

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

This code is inefficient because line 176 will construct vec by iterating the vector and then attr = vec will do an iterating copy.

Can you convert val to attr inplace so that it just iterates once?

Same as line 179 - line 181

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

explicit ExtractAttribute(const std::string& attr_name)
: attr_name_(attr_name) {}

std::vector<double>* operator()(Attribute& attr) const {
Copy link
Member

Choose a reason for hiding this comment

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

Just knowledge, not your fault. Our C++ style requires the change-able input parameter attr as a pointer, but I read the old code doesn't follow the style...

So you don have to change anything. It is just a kindly remind comment :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot.

Copy link
Contributor

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

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

LGTM for framework.py

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 for boost::get

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

@liym27 liym27 merged commit b4989fb into PaddlePaddle:develop Jan 11, 2021
liym27 added a commit to liym27/Paddle that referenced this pull request Jan 11, 2021
lanxianghit pushed a commit that referenced this pull request Jan 11, 2021
…et_value suppport vector<double> as value (#30126) (#30305)

Cherry-Pick #30126
1. Support vector<float64> as type of op attribute.
2. op set_value suppports float64 numpy.array
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.

5 participants