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

Change the invoking method of settiem from numpy to set_value op when value isn't tensor #35701

Merged
merged 8 commits into from
Sep 15, 2021

Conversation

zyfncg
Copy link
Contributor

@zyfncg zyfncg commented Sep 13, 2021

PR types

Others

PR changes

APIs

Describe

Change the invoking method of settiem from numpy to set_value op when value isn't tensor

当执行如下代码时:( 右边的值为非tensor)

tensor[0, :] = 1 

修改前的setitem由numpy完成底层逻辑处理
修改后的setitem由set_value op完成底层逻辑处理

@paddle-bot-old
Copy link

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


// cast numpy type form S to T, this may allocate new memory
template <class T, class S>
static py::array_t<T> cast_numpy_type(py::array_t<S> array) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里函数命名采用驼峰式
image

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.

}

template <class T>
static py::array_t<T> cast_numpy_array(const py::object &array) {
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.

"Currently, VarBase.__getitem__() only allows None or integers in "
"slice item, but received %s.",
"Currently, slice indices only allows None, integers, and "
"int of tensor or numpy in slice item, but received %s.",
Copy link
Contributor

Choose a reason for hiding this comment

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

int of 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.

"Currently, VarBase.__getitem__() only allows None or integers in "
"slice item, but received %s.",
"Currently, slice indices only allows None, integers, and "
"int of tensor or numpy in slice item, but received %s.",
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.

@@ -408,13 +500,36 @@ static int _PySlice_GetIndices(PySliceObject *r, Py_ssize_t length,
} else {
if (PyCheckInteger(r->stop) || IsNumpyType(r->stop)) {
*stop = PyLong_AsLong(r->stop);
} else if (PyCheckTensor(r->stop)) {
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. 已整合

@@ -554,7 +669,7 @@ static void ParseIndexingSlice(

} else {
PADDLE_THROW(platform::errors::InvalidArgument(
"Currently, VarBase.__getitem__() only allows indexing "
"Currently, VarBase.__indices__() only allows indexing "
Copy link
Contributor

Choose a reason for hiding this comment

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

VarBase可以改为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.

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 86d4af3 into PaddlePaddle:develop Sep 15, 2021
@zyfncg zyfncg deleted the zyf_slice branch September 15, 2021 09:38
AnnaTrainingG pushed a commit to AnnaTrainingG/Paddle that referenced this pull request Sep 29, 2021
… value isn't tensor (PaddlePaddle#35701)

* Change the invoking method of settiem from numpy to set_value op when value is not tensor

* fix the check logic for inplace in setitem

* fix the unittest problem caused by setitem doesn't support fp16

* modify some code format in setitem
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.

2 participants