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.

Sign up for GitHub

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

Jump to bottom

Feat empty op #5659

Merged
merged 59 commits into from Aug 7, 2021
Merged

Feat empty op #5659

merged 59 commits into from Aug 7, 2021

Conversation

wyg1997
Copy link
Contributor

@wyg1997 wyg1997 commented Jul 29, 2021 •
edited

本PR添加EmptyOp，使Tensor的创建统一通过Op。

文档：

image

@wyg1997 wyg1997 marked this pull request as draft July 29, 2021 08:42
@wyg1997 wyg1997 changed the title Feat add empty op Feat empty op Jul 29, 2021
@wyg1997 wyg1997 marked this pull request as ready for review July 29, 2021 12:43
@wyg1997 wyg1997 requested a review from lixinqi July 29, 2021 12:44
lixinqi
lixinqi reviewed Jul 30, 2021
View reviewed changes
oneflow/api/python/framework/tensor.cpp Outdated Show resolved Hide resolved
oneflow/api/python/framework/tensor.cpp Outdated Show resolved Hide resolved
oneflow/core/functional/functional_api.yaml Outdated Show resolved Hide resolved
lixinqi
lixinqi reviewed Aug 3, 2021
View reviewed changes
oneflow/core/functional/impl/array_functor.cpp Outdated Show resolved Hide resolved
oneflow/core/functional/impl/array_functor.cpp Outdated Show resolved Hide resolved
python/oneflow/test/modules/test_empty.py Outdated Show resolved Hide resolved
python/oneflow/nn/modules/empty.py Show resolved Hide resolved
@wyg1997 wyg1997 changed the base branch from refactor_eager_blob_object_shape to master August 4, 2021 02:02
wyg1997
wyg1997 commented Aug 4, 2021
View reviewed changes
oneflow/core/framework/op_interpreter/eager_mirrored_op_interpreter.cpp
Comment on lines +150 to +154
} else {
// output i is inplaced.
// check thread_local TensorMeta and tensor_impl TensorMeta.
CHECK_OR_RETURN(tensor_impl->tensor_meta()->shape() == output_tensor_metas->at(i)->shape());
CHECK_OR_RETURN(tensor_impl->tensor_meta()->dtype() == output_tensor_metas->at(i)->dtype());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

如果是inplace则直接检察infer的结果

oneflow/core/framework/op_interpreter/eager_mirrored_op_interpreter.cpp
Comment on lines +140 to +142
// using thread_local TensorMeta pointer if inplace.
// using tensor_impl TensorMeta pointer if not inplace.
return output_tensor_metas->at(i);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

非inplace时正常 infer 到 thread_local TensorMeta 中，inplace 时 infer 到实际的 tensor_impl 中

@oneflow-ci-bot oneflow-ci-bot removed their request for review August 6, 2021 20:20
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2021

Speed stats: 
GPU Name: GeForce GTX 1080 

PyTorch resnet50 time: 139.9ms (= 6997.1ms / 50, input_shape=[16, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 128.1ms (= 6403.9ms / 50, input_shape=[16, 3, 224, 224], backward is enabled)
Relative speed: 1.09 (= 139.9ms / 128.1ms)

PyTorch resnet50 time: 83.3ms (= 4165.7ms / 50, input_shape=[8, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 74.3ms (= 3714.1ms / 50, input_shape=[8, 3, 224, 224], backward is enabled)
Relative speed: 1.12 (= 83.3ms / 74.3ms)

PyTorch resnet50 time: 58.0ms (= 2898.9ms / 50, input_shape=[4, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 48.7ms (= 2436.3ms / 50, input_shape=[4, 3, 224, 224], backward is enabled)
Relative speed: 1.19 (= 58.0ms / 48.7ms)

PyTorch resnet50 time: 47.9ms (= 2393.7ms / 50, input_shape=[2, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 40.7ms (= 2034.1ms / 50, input_shape=[2, 3, 224, 224], backward is enabled)
Relative speed: 1.18 (= 47.9ms / 40.7ms)

PyTorch resnet50 time: 40.7ms (= 2032.9ms / 50, input_shape=[1, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 41.4ms (= 2068.7ms / 50, input_shape=[1, 3, 224, 224], backward is enabled)
Relative speed: 0.98 (= 40.7ms / 41.4ms)

@oneflow-ci-bot oneflow-ci-bot removed their request for review August 7, 2021 02:13
@oneflow-ci-bot oneflow-ci-bot self-requested a review August 7, 2021 02:14
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2021

CI failed, removing label automerge

@github-actions github-actions bot removed the automerge label Aug 7, 2021
@wyg1997 wyg1997 requested review from oneflow-ci-bot and removed request for oneflow-ci-bot August 7, 2021 03:09
@oneflow-ci-bot oneflow-ci-bot removed their request for review August 7, 2021 03:10
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot August 7, 2021 05:40
@oneflow-ci-bot oneflow-ci-bot self-requested a review August 7, 2021 08:10
@oneflow-ci-bot oneflow-ci-bot removed their request for review August 7, 2021 09:53
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot August 7, 2021 09:54
@oneflow-ci-bot oneflow-ci-bot self-requested a review August 7, 2021 11:30
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2021

Speed stats: 
GPU Name: GeForce GTX 1080 

PyTorch resnet50 time: 138.2ms (= 6910.7ms / 50, input_shape=[16, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 127.8ms (= 6392.2ms / 50, input_shape=[16, 3, 224, 224], backward is enabled)
Relative speed: 1.08 (= 138.2ms / 127.8ms)

PyTorch resnet50 time: 85.2ms (= 4261.5ms / 50, input_shape=[8, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 74.3ms (= 3715.6ms / 50, input_shape=[8, 3, 224, 224], backward is enabled)
Relative speed: 1.15 (= 85.2ms / 74.3ms)

PyTorch resnet50 time: 57.9ms (= 2896.4ms / 50, input_shape=[4, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 48.9ms (= 2445.8ms / 50, input_shape=[4, 3, 224, 224], backward is enabled)
Relative speed: 1.18 (= 57.9ms / 48.9ms)

PyTorch resnet50 time: 47.0ms (= 2350.2ms / 50, input_shape=[2, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 43.4ms (= 2170.3ms / 50, input_shape=[2, 3, 224, 224], backward is enabled)
Relative speed: 1.08 (= 47.0ms / 43.4ms)

PyTorch resnet50 time: 43.1ms (= 2156.3ms / 50, input_shape=[1, 3, 224, 224], backward is enabled)
OneFlow resnet50 time: 44.9ms (= 2244.4ms / 50, input_shape=[1, 3, 224, 224], backward is enabled)
Relative speed: 0.96 (= 43.1ms / 44.9ms)

@oneflow-ci-bot oneflow-ci-bot merged commit ed9b5a5 into master Aug 7, 2021
@oneflow-ci-bot oneflow-ci-bot deleted the feat-add_empty_op branch August 7, 2021 12:58
chengtbf
chengtbf reviewed Aug 13, 2021
View reviewed changes
oneflow/user/ops/constant_op.cpp
CHECK_OR_RETURN(ParseSbpParallelFromString(sbp_str, &sbp_parallel));
CHECK_OR_RETURN(
(sbp_parallel.has_split_parallel() && sbp_parallel.split_parallel().axis() == 0)
|| sbp_parallel.has_broadcast_parallel());
Copy link
Contributor

Choose a reason for hiding this comment

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

这个 CHECK 应该删掉？

chengtbf
chengtbf reviewed Aug 13, 2021
View reviewed changes
oneflow/user/ops/empty_op.cpp
CHECK_OR_RETURN(ParseSbpParallelFromString(sbp_str, &sbp_parallel));
CHECK_OR_RETURN(
(sbp_parallel.has_split_parallel() && sbp_parallel.split_parallel().axis() == 0)
|| sbp_parallel.has_broadcast_parallel());
Copy link
Contributor

Choose a reason for hiding this comment

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

还有这个

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@strint strint strint left review comments

@chengtbf chengtbf chengtbf left review comments

@hjchen2 hjchen2 hjchen2 left review comments

@lixinqi lixinqi lixinqi approved these changes

@oneflow-ci-bot oneflow-ci-bot Awaiting requested review from oneflow-ci-bot

Assignees
No one assigned
Labels
automerge feature need-simple-ci op
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@wyg1997 @lixinqi @strint @chengtbf @hjchen2 @Flowingsun007 @oneflow-ci-bot