-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add pil backend for vision transforms #28035
Conversation
Thanks for your contribution! |
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
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
@@ -18,14 +18,21 @@ | |||
import cv2 |
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.
lazy import cv2 here
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.
感谢提醒,在functional中改为了lazy_import,这个是单测,无需lazy_import。
Accordding to the code (the type of flip), flip the input image | ||
|
||
def _is_numpy_image(img): | ||
return isinstance(img, np.ndarray) and (img.ndim in {2, 3}) |
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.
Maybe add note, not process the 4D image
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.
done, thanks
0 : Flip vertically | ||
1 : Flip horizontally | ||
pic (PIL.Image|np.ndarray): Image to be converted to tensor. | ||
data_format (str, optional): Data format of img, should be 'HWC' or |
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.
the comments of data_format need more clearly for input image or returned image :
"Data format of returned image, ... "
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.
done, thanks
""" | ||
|
||
def __init__(self, transforms=[]): | ||
self.transforms = transforms | ||
def __init__(self, keys=None, backend='pil'): |
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.
有个全局的切换接口觉得更易用,而不是每个接口都加backend, functional的实现有同样的问题
@@ -18,14 +18,21 @@ | |||
import cv2 |
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.
paddle默认不安装cv2的话,这里的cv2依赖哪个特定的版本呢?
import失败的时候,需要提示用户手动安装特定的cv2版本。
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.
感谢提醒,在functional中改为了lazy_import,这个是单测,无需lazy_import。默认还是让用户使用最新版本的opencv。
def setUp(self): | ||
self.backend = self.set_backend() |
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.
get_backend
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.
done, thanks
@@ -40,146 +47,207 @@ def setUp(self): | |||
(400, 300, 3)) * 255).astype('uint8') | |||
cv2.imwrite(os.path.join(sub_dir, str(j) + '.jpg'), fake_img) | |||
|
|||
def set_backend(self): |
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.
get_backend
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.
done, thanks
|
||
|
||
class TestTransformsPIL(TestTransformsCV2): | ||
def set_backend(self): |
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.
get_backend
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.
done, thanks
|
||
|
||
@keepdims | ||
def resize(img, size, interpolation=1): | ||
def resize(img, size, interpolation='bilinear', backend='pil'): |
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.
这里的backend='pil',建议增加backend状态,可以简化backend切换的过程,比如:
默认设置为backend=None,表示采用缺省设置,然后增加
transforms.set_default_backend(backend)
transforms.get_default_backend()
transforms._defaut_backend = 'pil'
这样的话,用户在可以避免大量手动切换代码, 比如:
resize(..., backend=self.backend)
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
# try: | ||
data = f(data) | ||
if data is None: | ||
print('deubgggggggg:', f) |
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.
clean debug code
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.
done, thanks
In the other cases, tensors are returned without scaling. | ||
|
||
Args: | ||
size (int|list|tuple): Desired output size. If size is a sequence like |
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.
to_tensor的init和call函数都没有size参数
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.
updated, thanks
Pads the given PIL.Image on all sides with specified padding mode and fill value. | ||
|
||
Args: | ||
img (PIL.Image|np.array): Image to be padded. |
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.
下面实现支持np.array吗?
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.
已经更新,functional_pil.py,functional_cv.py,functonal_tensor.pil还不暴露给用户。暂时可以先关注functional.py中的注释和示例
img = np.pad(img, ((pad_top, pad_bottom), (pad_left, pad_right)), | ||
padding_mode) | ||
|
||
return Image.fromarray(img) |
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.
从实现来看,不支持np.array的输入,注意上面的comments是否正确
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.
已经更新,functional_pil.py,functional_cv.py,functonal_tensor.pil还不暴露给用户。暂时可以先关注functional.py中的注释和示例
经过讨论,backend参数实际上只起到double check的作用,如果用户传入的图片类型和backend不符就直接报错了,而且这样的check也会降低性能。最后决定去除backend参数,根据用户传入的图片类型,调用相应backend的算子。同时,在读取图片的地方,增加了set_image_backend和get_image_backend函数,来控制想要的图片类型。 |
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.
utils.py may change to image.py or image_io.py
done, thanks |
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
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
return Image.open(path) | ||
else: | ||
cv2 = try_import('cv2') | ||
return cv2.imread(path) |
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.
这段代码可以重新组织下,比如:
if backend == 'pil':
...
elif backend == 'cv2':
...
else
raise ValueError
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.
嗯嗯,这个后续修改一下。目前来看并不影响
def to_tensor(pic, data_format='CHW'): | ||
"""Converts a ``PIL.Image`` or ``numpy.ndarray`` to paddle.Tensor. | ||
|
||
See ``ToTensor`` for more details. |
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.
最好写ToTensor的全路径
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.
好的,文档会在下个pr优化一下
if _is_pil_image(pic): | ||
return F_pil.to_tensor(pic, data_format) | ||
else: | ||
return F_cv2.to_tensor(pic, data_format) |
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.
从np array转成tensor的功能是放在了functional_cv2.py 下呀。
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.
是的,都是从input的数据类型来区分的
if _is_pil_image(img): | ||
return F_pil.pad(img, padding, fill, padding_mode) | ||
else: | ||
return F_cv2.pad(img, padding, fill, padding_mode) |
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.
这里的逻辑(及其他类似的对F_cv2
的调用逻辑),有些问题。
用户设置的backend是pillow的时候,传入的img
是np array时,走到了lazy import cv2的分支里。
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.
@XiaoguangHu01 @saxon-zh
麻烦看看这个问题,是否要改。
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.
线下讨论解决
PR types
New features
PR changes
APIs
Describe
文档地址:http://gzhxy-inf-bce36a39de.gzhxy.baidu.com:8121/documentation/docs/en/api/paddle/vision/BaseTransform_en.html