-
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
【Hackathon 6th No.10】Add isin API to Paddle -part #64001
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
>>> print(res) | ||
Tensor(shape=[5], dtype=bool, place=Place(cpu), stop_gradient=True, | ||
[True, False, False, True, False]) | ||
""" |
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.
参考 pytorch 在 _decompose 中的设计:当 test_elements 元素个数较少时直接进行暴力搜索,较多时则采取基于排序的算法:若 assume_unique=True,则用稳定排序的 argsort 进行实现;若 assume_unique=False,则用 searchsorted 进行实现。
示例代码中,能否体现出 assume_unique=True/False 的区别?注释中是否要加入这段说明
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.
增加了一个 assume_unique
设置错误导致结果出错的例子;
在下面代码中加了不同分支对应不同做法的注释
python/paddle/tensor/math.py
Outdated
@@ -7871,3 +7871,176 @@ def isreal(x, name=None): | |||
return paddle.ones_like(x, dtype='bool') | |||
|
|||
return paddle.equal(paddle.imag(x), 0) | |||
|
|||
|
|||
def isin(elements, test_elements, assume_unique=False, invert=False, name=None): |
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.
Accroding to API design guidelines standard, use x
instead of elements
and test_x
instead of test_elements
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.
revised.
'float32', | ||
'float64', | ||
'int32', | ||
'int64', |
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.
should we support bfloat16
and float16
?
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.
Temporarily not since paddle.searchsorted
only support 'float32', 'float64', 'int32' and 'int64'.
'float32', | ||
'float64', | ||
'int32', | ||
'int64', |
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.
same date type issue as above
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.
Temporarily not since paddle.searchsorted
only support 'float32', 'float64', 'int32' and 'int64'.
@jeff41404 |
the logic of searchsorted also supports comparing two inputs with different data types. Registering the fp16 and bf16 data types in the Operator should support these two data types. |
Got it. Let me update the |
Sorry to inform you that ce38eb2's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually. |
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 for docs
PR Category
User Experience
PR Types
New features
Description
rfcs: PaddlePaddle/community#884