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

integrate nn.image.flip #5411

Merged
merged 22 commits into from
Jul 13, 2021
Merged

integrate nn.image.flip #5411

merged 22 commits into from
Jul 13, 2021

Conversation

duchengyao
Copy link
Contributor

@duchengyao duchengyao commented Jul 7, 2021 •
edited
Loading

image

@duchengyao duchengyao changed the title init nn.image.flip integrate nn.image.flip Jul 7, 2021
@duchengyao duchengyao mentioned this pull request Jul 7, 2021
Closed
simonJJJ
simonJJJ reviewed Jul 7, 2021
View reviewed changes
oneflow/python/test/modules/test_image_flip.py Outdated
assert all([len(image.shape) == 4 for image in images])

image_static_shape = _get_images_static_shape(images)
flip_codes = flow.Tensor([1, 1], dtype=flow.int8)
Copy link
Contributor

Choose a reason for hiding this comment

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

为何这里的flip_codes是[1, 1]? 应该直接传一个1, 在Module内部根据多少张image将它扩成[1, 1]

oneflow/python/nn/modules/dataset.py
>>> image_tensor_buffer = flow.tensor_to_tensor_buffer(image_tensors, instance_dims=3)
>>> flip_codes = flow.Tensor([1,1],dtype=flow.int8)
>>> output = nn.image.flip()(image_tensor_buffer, flip_codes)
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

这里发一下文档和doctest的截图

oneflow/python/nn/modules/dataset.py Outdated
self._op = (
flow.builtin_op("image_flip")
.Input("in")
.Input("flip_code")
Copy link
Contributor

Choose a reason for hiding this comment

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

建议将flip_code初始化就传进来，而不是forward的时候，这样更符合Module

Copy link
Contributor

Choose a reason for hiding this comment

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

把这个op放到functor里面去

Copy link
Contributor Author

Choose a reason for hiding this comment

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

建个dataset_functor.cpp吗？ @BBuf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#5402 这个为啥不是functor的，有没有跟image/dataset有关的functor我可以抄一下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@simonJJJ @BBuf

simonJJJ
simonJJJ reviewed Jul 9, 2021
View reviewed changes
oneflow/core/functional/impl/dataset_functor.cpp
@@ -0,0 +1,36 @@
#include "oneflow/core/framework/op_builder.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

这里没有format

Copy link
Contributor Author

Choose a reason for hiding this comment

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

你能先帮我看看怎么把代码跑起来吗

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.

谢谢大佬

@simonJJJ simonJJJ requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 12, 2021 06:40
@oneflow-ci-bot oneflow-ci-bot removed their request for review July 12, 2021 07:08
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 12, 2021 08:30
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 12, 2021 09:43
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 12, 2021 11:58
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 12, 2021 13:14
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 12, 2021 14:42
@oneflow-ci-bot oneflow-ci-bot removed their request for review July 13, 2021 02:50
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 13, 2021 03:51
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot July 13, 2021 05:06
@oneflow-ci-bot oneflow-ci-bot merged commit c68c47d into master Jul 13, 2021
@oneflow-ci-bot oneflow-ci-bot deleted the image.flip branch July 13, 2021 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BBuf BBuf BBuf left review comments

@simonJJJ simonJJJ simonJJJ approved these changes

Assignees
No one assigned
Labels
automerge feature interface python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@duchengyao @simonJJJ @BBuf @oneflow-ci-bot