Add searchsorted op #35159

Yanxing-Shi · 2021-08-25T15:50:12Z

PR types

New features

PR changes

OPs

Describe

Add paddle.searchsorted op

CLAassistant · 2021-08-25T15:50:15Z

All committers have signed the CLA.

paddle-bot-old · 2021-08-25T15:50:15Z

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

sneaxiy

Excellent job with tiny comments.

sneaxiy · 2021-08-26T12:04:21Z

python/paddle/tensor/search.py

+     - ``sorted_sequence[m][n]...[l][i-1] <= values[m][n]...[l][x] < sorted_sequence[m][n]...[l][i]``
+
+    Args:
+    sorted_sequence(Tensor): N-D or 1-D tensor, containing monotonically increasing sequence on the innermost dimension. The data type can be int32, int64, float32, float64.


Add indent here. Same below.
加个缩进，下同。

sneaxiy · 2021-08-26T12:04:53Z

python/paddle/tensor/search.py

+            refer to :ref:`api_guide_Name`.
+
+    Returns:
+    output (Tensor):return the indices from the innermost dimension of sorted_sequence. The output tensor is the same size as values.


Add indent here.

sneaxiy · 2021-08-26T12:07:28Z

python/paddle/fluid/tests/unittests/test_searchsorted_op.py

+import paddle.nn.functional as F
+import paddle.fluid as fluid
+import paddle.fluid.core as core
+from paddle.fluid import compiler, Program, program_guard


Not all imports are necessary. Please remove useless import expressions.

sneaxiy · 2021-08-26T12:09:09Z

python/paddle/fluid/tests/unittests/test_searchsorted_op.py

+                              fetch_list=out)
+            out_ref = np.searchsorted(self.sorted_sequence, self.values)
+            for r in res:
+                self.assertEqual(np.allclose(out_ref, r), True)


The res is a list with only one element. Please remove the for loop.

Use self.assertTrue(a) instead of self.assertEqual(a, True).

sneaxiy · 2021-08-26T12:10:31Z

python/paddle/fluid/tests/unittests/test_searchsorted_op.py

+
+            paddle.disable_static(place)
+            SortedSequence = paddle.to_tensor(self.sorted_sequence)
+            Values = paddle.to_tensor(self.values)


Try to use sorted_sequence/values instead of SearchSequence/Values. The snake case naming rule is more Pythonic.

sneaxiy · 2021-08-26T12:10:58Z

python/paddle/tensor/__init__.py

           'round_',
           'rsqrt',
           'rsqrt_',
+           'searchsorted',


Why add this? Remove it.

sneaxiy · 2021-08-26T12:13:37Z

paddle/fluid/operators/searchsorted_op.cc

+                                        "dimensions of sorted_sequence tensor "
+                                        "and input values tensor must "
+                                        "match, but we got sorted_sequence "
+                                        "tensor ( %s ), and input value "


input value -> input values

sneaxiy · 2021-08-26T12:17:17Z

paddle/fluid/operators/searchsorted_op.cc

+#include <memory>
+#include <string>
+#include <unordered_map>
+#include <vector>


I do not think that you should include these headers. Useless below.

sneaxiy · 2021-08-26T12:17:31Z

paddle/fluid/operators/math/algorithm.h


 template <typename T>
-HOSTDEVICE inline int64_t BinarySearch(const T *x, int64_t num, const T &val) {
+HOSTDEVICE inline int64_t BinarySearch(const T *x, size_t num, const T &val) {


No need to change. Keep int64_t.

sneaxiy · 2021-08-26T12:18:02Z

paddle/fluid/operators/searchsorted_op.cc

+    auto values_dims = ctx->GetInputDim("Values");
+    auto out_int32 = ctx->Attrs().Get<bool>("out_int32");
+
+    if (sequences_dims.size() != 1)


Add brackets after if (...). It would be more readable.

sneaxiy

LGTM for codes. Reviewing docs ...

sneaxiy

LGTM for codes but some docs need to be updated.

sneaxiy · 2021-09-07T12:19:58Z

python/paddle/tensor/search.py

+
+    Args:
+        sorted_sequence(Tensor): An input N-D or 1-D tensor with type int32, int64, float32, float64. The value of the tensor monotonically increases in the innermost dimension. 
+        values(Tensor or Scalar): An input N-D tensor or a Scalar value with type int32, int64, float32, float64.


No Scalar. Paddle does not support Scalar.

sneaxiy · 2021-09-07T12:20:22Z

python/paddle/tensor/search.py

+        sorted_sequence(Tensor): An input N-D or 1-D tensor with type int32, int64, float32, float64. The value of the tensor monotonically increases in the innermost dimension. 
+        values(Tensor or Scalar): An input N-D tensor or a Scalar value with type int32, int64, float32, float64.
+        out_int32(bool, optional): Data type of the output tensor which can be int32, int64. The default value is False, and it indicates that the output data type is int64.
+        right(bool, optional): Find the upper and lower bounds of the sorted_sequence range in the innermost dimension based on the given `values`. If the sorted_sequence value is nan or inf, return the size of the innermost dimension


upper and lower bounds -> upper or lower bounds.

return the size of the innermost dimension -> return the size of the innermost dimension.

sneaxiy · 2021-09-07T12:21:33Z

python/paddle/tensor/search.py

+        right(bool, optional): Find the upper and lower bounds of the sorted_sequence range in the innermost dimension based on the given `values`. If the sorted_sequence value is nan or inf, return the size of the innermost dimension
+                               The default value is False and it shows the lower bounds.  
+        name(str, optional): The default value is None. Normally there is no need for user to set this property. For more information, please
+                             refer to :ref:`api_guide_Name`.


Put this line to the previous line.

sneaxiy

LGTM now.

qingqing01

LGTM for API

qingqing01

LGTM for API

qingqing01

LGTM for API.

TCChenlong

LGTM

XiaoguangHu01

LG API

jzhang533

LGTM

* fix github name * fix CI error * fix review and CI error * fix inf,nan error and modify unittest samples * add unittest samples * add unittest samples * fix unittest error * test=document_fix * test=document_fix * modify doc and add unittest samples * fix error newline in constant * modify doc after mentor review * modify __all__ and doc * modify doc

Yanxing-Shi added 2 commits August 26, 2021 02:29

fix github name

cd76fb5

fix CI error

35b2dc9

sneaxiy reviewed Aug 26, 2021

View reviewed changes

fix review and CI error

f154c93

Yanxing-Shi requested a review from sneaxiy August 27, 2021 11:36

Yanxing-Shi marked this pull request as draft August 27, 2021 11:41

Yanxing-Shi added 8 commits August 30, 2021 05:56

fix inf,nan error and modify unittest samples

7f44f54

add unittest samples

b28edf3

add unittest samples

cc92a71

fix unittest error

2f49f8b

test=document_fix

907e71f

test=document_fix

32bdd22

modify doc and add unittest samples

f21b24a

fix error newline in constant

753e3e1

sneaxiy requested review from XiaoguangHu01, Xreki, jzhang533, lanxianghit and qingqing01 September 6, 2021 06:46

Yanxing-Shi marked this pull request as ready for review September 7, 2021 12:17

sneaxiy reviewed Sep 7, 2021

View reviewed changes

modify doc after mentor review

0fcc53d

sneaxiy previously approved these changes Sep 8, 2021

View reviewed changes

qingqing01 reviewed Sep 8, 2021

View reviewed changes

qingqing01 previously approved these changes Sep 8, 2021

View reviewed changes

modify __all__ and doc

33c5996

Yanxing-Shi dismissed stale reviews from qingqing01 and sneaxiy via 33c5996 September 9, 2021 08:22

Yanxing-Shi mentioned this pull request Sep 9, 2021

add searchsorted_cn.rst doc PaddlePaddle/docs#3866

Closed

Yanxing-Shi added 2 commits September 9, 2021 11:52

modify doc

ed2173d

Merge remote-tracking branch 'upstream/develop' into add_searchsorted_op

c645624

Yanxing-Shi mentioned this pull request Sep 10, 2021

add paddle.searchsorted doc PaddlePaddle/docs#3867

Merged

sneaxiy approved these changes Sep 10, 2021

View reviewed changes

qingqing01 approved these changes Sep 10, 2021

View reviewed changes

lanxianghit approved these changes Sep 10, 2021

View reviewed changes

TCChenlong approved these changes Sep 10, 2021

View reviewed changes

XiaoguangHu01 approved these changes Sep 10, 2021

View reviewed changes

jzhang533 approved these changes Sep 13, 2021

View reviewed changes

sneaxiy merged commit 6622304 into PaddlePaddle:develop Sep 13, 2021

Yanxing-Shi deleted the add_searchsorted_op branch November 9, 2021 02:43

Add searchsorted op #35159

Add searchsorted op #35159

Uh oh!

Conversation

Yanxing-Shi commented Aug 25, 2021

PR types

PR changes

Describe

Uh oh!

CLAassistant commented Aug 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paddle-bot-old bot commented Aug 25, 2021

Uh oh!

sneaxiy left a comment

Choose a reason for hiding this comment

Uh oh!

sneaxiy Aug 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sneaxiy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sneaxiy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sneaxiy left a comment

Choose a reason for hiding this comment

Uh oh!

qingqing01 left a comment

Choose a reason for hiding this comment

Uh oh!

qingqing01 left a comment

Choose a reason for hiding this comment

Uh oh!

qingqing01 left a comment

Choose a reason for hiding this comment

Uh oh!

TCChenlong left a comment

Choose a reason for hiding this comment

Uh oh!

XiaoguangHu01 left a comment

Choose a reason for hiding this comment

Uh oh!

jzhang533 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

CLAassistant commented Aug 25, 2021 •

edited

Loading

sneaxiy Aug 26, 2021 •

edited

Loading

sneaxiy left a comment •

edited

Loading