Add flip module #5541

BBuf · 2021-07-19T09:27:23Z

添加flow.flip module，对齐torch.flip。

添加flip op和kernel
添加functor
添加gradient_funcs的后向实现
实现module，导出flow.flip和tensor.flip
添加docstting和单元测试以及doctest
修改review意见

docs截图：

doctest截图：

Co-authored-by: oneflow-ci-bot <69100618+oneflow-ci-bot@users.noreply.github.com>

Flowingsun007 · 2021-07-19T13:05:57Z

oneflow/python/nn/modules/flip.py

+class Flip(Module):
+    def __init__(self, dims) -> None:
+        super().__init__()
+        assert isinstance(dims, list) or isinstance(


可以写成：assert isinstance(dims, (list,tuple))

Flowingsun007 · 2021-07-19T13:23:17Z

oneflow/python/nn/modules/flip.py

+
+
+class Flip(Module):
+    def __init__(self, dims) -> None:


这里dims没有参数类型检查

Flowingsun007 · 2021-07-19T13:33:00Z

oneflow/user/kernels/flip_kernel.cu

+
+template<typename T>
+__global__ void FlipGpuForward(const int32_t element, const int64_t total_dims,
+                               const STRIDE_CONTIGUOUS_V stride_contiguous_v, const SIZE_V sizes_v,


这里为啥不直接传const std::vector<int32_t>，而要构造STRIDE_CONTIGUOUS_V、SIZE_V、STRIDE？

cuda不能用stl这些结构，我沿用之前其它pr的做法

好的，不过STRIDE_CONTIGUOUS_V、SIZE_V、STRIDE的结构都一样，应该复用一个结构体就行（不需要构造3次）

Flowingsun007 · 2021-07-19T13:44:35Z

oneflow/user/kernels/flip_kernel.cpp

+}  // namespace
+
+template<typename T>
+class FlipCpuKernel final : public user_op::OpKernel {


kernel包在user_op的namespace下吧

Flowingsun007 · 2021-07-19T13:49:51Z

oneflow/core/functional/impl/array_functor.cpp

+ public:
+  FlipFunctor() { op_ = CHECK_JUST(one::OpBuilder("flip").Input("x").Output("y").Build()); }
+  Maybe<Tensor> operator()(const std::shared_ptr<one::Tensor>& x,
+                           const std::vector<int32_t> dims) const {


Suggested change

const std::vector<int32_t> dims) const {

const std::vector<int32_t>& dims) const {

Flowingsun007 · 2021-07-19T13:50:35Z

oneflow/core/functional/impl/array_functor.cpp

+    op_ = CHECK_JUST(one::OpBuilder("flip_grad").Input("dy").Output("dx").Build());
+  }
+  Maybe<Tensor> operator()(const std::shared_ptr<one::Tensor>& dy,
+                           const std::vector<int32_t> dims) const {


Suggested change

const std::vector<int32_t> dims) const {

const std::vector<int32_t>& dims) const {

Flowingsun007 · 2021-07-19T13:56:04Z

oneflow/user/ops/flip_op.cpp

+    .SetTensorDescInferFn([](user_op::InferContext* ctx) -> Maybe<void> {
+      const user_op::TensorDesc* x_desc = ctx->TensorDesc4ArgNameAndIndex("x", 0);
+      user_op::TensorDesc* y_desc = ctx->OutputTensorDesc("y", 0);
+      *y_desc->mut_shape() = x_desc->shape();


这里需要check一下dims参数是否<=input tensor的维度

好的，应该是小于不能等于。

torch的可以等于

我意思是dims列表的长度<=x的维度数；你说的是具体到某个dim

两者应该都需要检查

Flowingsun007 · 2021-07-19T13:58:07Z

oneflow/python/nn/modules/flip.py

+        self.dims = dims
+
+    def forward(self, x):
+        return flow.F.flip(x, self.dims)


python层同样需要加一下dims检查，譬如：IndexError: Dimension out of range (expected to be in range of xxx, but got xxx)

Flowingsun007 · 2021-07-19T14:10:09Z

oneflow/user/kernels/flip_kernel.cpp

+    SIZE_V sizes_v;
+    for (int32_t i = 0; i < total_dims; i++) { sizes_v.val[i] = y_tensor->shape().At(i); }
+
+    STRIDE strides_v;


标记一下TODO吧（等stride支持以后，这段逻辑就可以不用了）

Flowingsun007 · 2021-07-19T14:19:31Z

oneflow/user/kernels/flip_kernel.cu

+      int32_t temp = cur_indices;
+      cur_indices = cur_indices / stride_contiguous_v.val[d];
+      rem = temp - cur_indices * stride_contiguous_v.val[d];
+      dst_offset += vis.val[d] ? (sizes_v.val[d] - 1 - cur_indices) * strides_v.val[d]


这块逻辑和torch好像没对齐？

为什么没对齐呢

into add_flip_module

BBuf and others added 19 commits June 29, 2021 21:41

fix upsample nearest bug

34ee0a6

Merge branch 'master' of https://github.com/Oneflow-Inc/oneflow

0219f8f

Merge branch 'master' of https://github.com/Oneflow-Inc/oneflow

c822046

Merge branch 'master' of https://github.com/Oneflow-Inc/oneflow

0106b53

fix upsample nearest bug (#5347)

267d4ca

Co-authored-by: oneflow-ci-bot <69100618+oneflow-ci-bot@users.noreply.github.com>

fix upsample bilinear bug

205c7fb

Merge branch 'master' of https://github.com/Oneflow-Inc/oneflow

38f6dfd

fix conflict

318ef54

Merge branch 'master' of https://github.com/Oneflow-Inc/oneflow

544e107

Merge branch 'master' of https://github.com/Oneflow-Inc/oneflow

5f98a22

Merge branch 'master' of https://github.com/Oneflow-Inc/oneflow

3bde816

init flip op

7ab829c

add flip op register

9b3bc46

add flip cpu kernel forward

a1c590e

add flip kernel impl

38ad216

add flip op functor and gradient_funcs

684e143

add test, still need fix bug

6c1a600

fix segmentfault bug

6ab2a90

add docs

8d616b6

BBuf requested review from doombeaker and hjchen2 July 19, 2021 09:27

Merge branch 'master' into add_flip_module

a4c429b

BBuf requested a review from Flowingsun007 July 19, 2021 10:05

Flowingsun007 reviewed Jul 19, 2021

View reviewed changes

oneflow/python/nn/modules/flip.py

class Flip(Module):

def __init__(self, dims) -> None:

Copy link

Contributor

Flowingsun007 Jul 19, 2021

Choose a reason for hiding this comment

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

这里dims没有参数类型检查

Flowingsun007 reviewed Jul 19, 2021

View reviewed changes

fix comments

4e41bd6

Flowingsun007 approved these changes Jul 20, 2021

View reviewed changes

BBuf and others added 2 commits July 20, 2021 10:20

fix comments

57b66f5

Merge branch 'master' into add_flip_module

0b0759a

BBuf added api automerge documentation eager enhancement feature labels Jul 20, 2021

BBuf requested a review from oneflow-ci-bot July 20, 2021 02:22

BBuf added 2 commits July 20, 2021 10:39

fix comments

70ad605

Merge branch 'add_flip_module' of https://github.com/Oneflow-Inc/oneflow

30a11df

into add_flip_module

BBuf requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 20, 2021 02:47

oneflow-ci-bot removed their request for review July 20, 2021 04:26

Merge branch 'master' into add_flip_module

c9f9a1b

oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 20, 2021 04:26

oneflow-ci-bot merged commit 2b208ec into master Jul 20, 2021

oneflow-ci-bot deleted the add_flip_module branch July 20, 2021 07:29

BBuf mentioned this pull request Jul 21, 2021

OneFlow 算子对齐 PyTorch 完备计划推进表 #4936

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flip module #5541

Add flip module #5541

BBuf commented Jul 19, 2021 •

edited

Loading

Flowingsun007 Jul 19, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 19, 2021

Flowingsun007 Jul 19, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 20, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 19, 2021

Flowingsun007 Jul 19, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 19, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 19, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 20, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 20, 2021

Flowingsun007 Jul 20, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 19, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 19, 2021

BBuf Jul 20, 2021

Flowingsun007 Jul 19, 2021

BBuf Jul 20, 2021

	const std::vector<int32_t> dims) const {
	const std::vector<int32_t>& dims) const {

Add flip module #5541

Add flip module #5541

Conversation

BBuf commented Jul 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BBuf commented Jul 19, 2021 •

edited

Loading