-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Forward chainerx::MakeArray in some case #8296
Conversation
Could you explain a bit more? For what case this fix is needed? |
Updated the PR comment |
chainerx_cc/chainerx/python/array.cc
Outdated
if (!copy && py::isinstance<ArrayBody>(object)) { | ||
ArrayBodyPtr body = py::cast<ArrayBodyPtr>(object); | ||
if ((device.is_none() || &dev == &body->device()) && (dtype.is_none() || *dtype_ == body->dtype())) { | ||
return std::move(body); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you don't have to move
the return value.
@@ -259,10 +259,14 @@ def test_array_from_chainerx_array_with_device( | |||
|
|||
dst_device = chainerx.get_device(dst_device_spec) | |||
|
|||
if not copy and src_dtype == dst_dtype and device is dst_device: | |||
if not copy and \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's follow the recommendation in PEP-8.
- Use parentheses instead of
\
. - Place
and
after line breaks.
I think |
Sorry never mind, the above comment only assumed the default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Jenkins, test this please |
Jenkins CI test (for commit d5a9a6b, target branch master) failed with status FAILURE. |
Jenkins, test this please |
Jenkins CI test (for commit d5a9a6b, target branch master) succeeded! |
LGTM @emcastillo |
Spin-offed from: #8262
In non-copy mode of more C++ like
MakeArray
if device and dtype matches the source array so python like API should follow the behavior too:chainer/chainerx_cc/chainerx/python/array.cc
Lines 153 to 155 in 49b6941
TODO: