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

Prune python tensor #5596

Merged
merged 22 commits into from
Jul 28, 2021
Merged

Prune python tensor #5596

merged 22 commits into from
Jul 28, 2021

Conversation

daquexian
Copy link
Contributor

@daquexian daquexian commented Jul 26, 2021

将 python 的 Tensor 类删除,逻辑转移到 c++

>>> import oneflow as flow
>>> x=flow.Tensor(1,2)
>>> type(x)
<class 'oneflow.Tensor'>
>>> y=x+x
>>> type(y)
<class 'oneflow.Tensor'>
>>>

Signed-off-by: daquexian <daquexian566@gmail.com>
Signed-off-by: daquexian <daquexian566@gmail.com>
Signed-off-by: daquexian <daquexian566@gmail.com>
Signed-off-by: daquexian <daquexian566@gmail.com>
@@ -13,55 +13,24 @@
See the License for the specific language governing permissions and
limitations under the License.
"""
from oneflow.compatible.single_client.core.job import (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

用 oneflow/python/framework/tensor.py 的内容覆盖了这个文件的原内容。因为它们共用了一个 _oneflow_internal


namespace oneflow {
namespace pyext {

namespace {
static PyObject* py_kernels_dic = nullptr;

void OFDataTypeToNumpyType(DataType of_data_type, int* out_numpy_type) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

这两个方法移动到了 oneflow/extension/python/numpy.h 里并做了修改

Signed-off-by: daquexian <daquexian566@gmail.com>
@daquexian daquexian requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 26, 2021 03:19
Signed-off-by: daquexian <daquexian566@gmail.com>
Tensor.normal_ = _normal
Tensor.fill_ = _fill
Tensor._placement_scope = _placement_scope
Tensor.copy_ = _copy
Copy link
Contributor Author

Choose a reason for hiding this comment

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

c++ tensor 类的一些方法在 python 层注入,暂时不着急全部重写到 c++

Signed-off-by: daquexian <daquexian566@gmail.com>
@daquexian daquexian requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 26, 2021 03:53
@oneflow-ci-bot oneflow-ci-bot removed their request for review July 26, 2021 05:16
oneflow/api/python/framework/device.cpp Outdated Show resolved Hide resolved
oneflow/api/python/framework/device.cpp Outdated Show resolved Hide resolved
oneflow/api/python/framework/tensor.cpp Outdated Show resolved Hide resolved
Signed-off-by: daquexian <daquexian566@gmail.com>
Signed-off-by: daquexian <daquexian566@gmail.com>
@@ -1,7 +1,7 @@
include(FetchContent)
set(PYBIND11_TAR_URL https://github.com/pybind/pybind11/archive/v2.6.0.zip)
set(PYBIND11_TAR_URL https://github.com/pybind/pybind11/archive/v2.7.0.zip)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

升级 pybind11 把这个修复 gcc 4 编译错误的 pr 包含进来 pybind/pybind11#2956

@oneflow-ci-bot oneflow-ci-bot removed their request for review July 26, 2021 07:09
Copy link
Contributor

@wyg1997 wyg1997 left a comment

Choose a reason for hiding this comment

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

Parameter的构造函数里是不是忘处理了?

} else {
return MakeLocalTensorByNumpy(arg, desired_dtype, device, requires_grad);
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里需要和Pytorch那样,如果参数检察不正确,把所有的合法参数都打印出来给个提示吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

感觉可以 😂

Signed-off-by: daquexian <daquexian566@gmail.com>
Signed-off-by: daquexian <daquexian566@gmail.com>
@oneflow-ci-bot oneflow-ci-bot removed their request for review July 27, 2021 11:15
Comment on lines +27 to +31
std::string type;
int device_id = -1;
ParsingDeviceTag(type_and_id, &type, &device_id).GetOrThrow();
if (device_id == -1) { device_id = 0; }
return MakeDevice(type, device_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

记得新开一个issue,让新手对这里做优化。用thread_local作缓存。

}

bool requires_grad() const override { return tensor_->requires_grad(); }
bool is_leaf() const override { return tensor_->is_leaf(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

是不是应该直接返回true呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,已修改,同时通过让 Parameter 继承 TensorIf,也实现了 Parameter 有独立的 grad_fn

user_op::TensorDesc* mut_tensor_meta() override { return tensor_->mut_tensor_meta(); }

Maybe<MirroredTensor> AsMirroredTensor() override {
if (const auto& mirrored_tensor = std::dynamic_pointer_cast<MirroredTensor>(tensor_)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这语法是c++11所能支持的吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,这个和 c++17 的 if (int x = 0; x < 1) 不是同一个语法

Copy link
Contributor

Choose a reason for hiding this comment

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

哦哦。我才注意到两者的区别,那我还把很多的宏写复杂了。

oneflow/api/python/framework/tensor.cpp Outdated Show resolved Hide resolved
oneflow/api/python/framework/tensor.cpp Outdated Show resolved Hide resolved
Signed-off-by: daquexian <daquexian566@gmail.com>
Signed-off-by: daquexian <daquexian566@gmail.com>
@daquexian daquexian requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 27, 2021 14:34
@daquexian daquexian requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 27, 2021 14:37
@daquexian daquexian mentioned this pull request Jul 27, 2021
@oneflow-ci-bot oneflow-ci-bot removed their request for review July 27, 2021 16:02
@github-actions
Copy link
Contributor

Speed stats:
GPU Name: GeForce GTX 1080 

PyTorch resnet50 time: 141.0ms (= 7050.6ms / 50, input_shape=[16, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 126.3ms (= 6313.8ms / 50, input_shape=[16, 3, 224, 224], backward is enabled)
Relative speed: 1.12 (= 141.0ms / 126.3ms)

PyTorch resnet50 time: 84.7ms (= 4236.8ms / 50, input_shape=[8, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 73.7ms (= 3683.9ms / 50, input_shape=[8, 3, 224, 224], backward is enabled)
Relative speed: 1.15 (= 84.7ms / 73.7ms)

PyTorch resnet50 time: 60.3ms (= 3016.7ms / 50, input_shape=[4, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 49.2ms (= 2459.6ms / 50, input_shape=[4, 3, 224, 224], backward is enabled)
Relative speed: 1.23 (= 60.3ms / 49.2ms)

PyTorch resnet50 time: 51.2ms (= 2558.2ms / 50, input_shape=[2, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 45.7ms (= 2286.1ms / 50, input_shape=[2, 3, 224, 224], backward is enabled)
Relative speed: 1.12 (= 51.2ms / 45.7ms)

PyTorch resnet50 time: 44.0ms (= 2199.3ms / 50, input_shape=[1, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 41.9ms (= 2094.6ms / 50, input_shape=[1, 3, 224, 224], backward is enabled)
Relative speed: 1.05 (= 44.0ms / 41.9ms)

@oneflow-ci-bot oneflow-ci-bot removed their request for review July 28, 2021 02:04
@oneflow-ci-bot oneflow-ci-bot merged commit 3a2b338 into master Jul 28, 2021
@oneflow-ci-bot oneflow-ci-bot deleted the prune_python_tensor branch July 28, 2021 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants