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.

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

[PaddlePaddle hackathon] Add randint_like #36169

Merged
merged 22 commits into from
Nov 2, 2021

Conversation

JunnYu
Copy link
Member

@JunnYu JunnYu commented Sep 27, 2021

PR types

New features

PR changes

OPs

Describe

@paddle-bot-old
Copy link

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

@JunnYu JunnYu changed the title Add randint_like [PaddlePaddle hackathon] Add randint_like Sep 28, 2021
@JunnYu
Copy link
Member Author

JunnYu commented Sep 28, 2021

  • PR-CI-Windows 好多都是超时(与我修改的代码无关)。
  • PR-CI-OP-benchmark [ERROR] Missing test script of "randint_like"(paddle/fluid/operators/randint_like_op.cu) in benchmark(测试代码使用python写了。)
  • PR-CI-APPROVAL和PR-CI-CPU-Py2没有approve。

Copy link
Contributor

@zhiboniu zhiboniu left a comment

Choose a reason for hiding this comment

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

实现一个API的时候,并不急于新增对应的op,可以先查一下已有op是否满足使用要求,如果可以满足使用要求,要尽量复用已有的op,避免堆积重复的功能。
你可以检查看看类似功能的API使用的op

@JunnYu
Copy link
Member Author

JunnYu commented Oct 11, 2021

@zhiboniu 好的,那我就用纯python来实现这个api了。

@JunnYu JunnYu requested a review from zhiboniu October 13, 2021 16:26
python/paddle/__init__.py Show resolved Hide resolved
python/paddle/tensor/random.py Outdated Show resolved Hide resolved
python/paddle/tensor/random.py Outdated Show resolved Hide resolved
@JunnYu
Copy link
Member Author

JunnYu commented Oct 15, 2021

@zhiboniu 请问所需的是哪种randint like

  • 1 当前代码,只使用了输入x的shape,未使用x的dtype。输出的dtype只可以在[int32,int64]中选择。
  • 2 只使用输入x的shape, 输出的dtype可以从[int32,int64,float16,float32,float64]中选择。
  • 3 使用x的shape,当dtype不是None的时候,输出指定类型的dtype。否则输出的dtype与输入的dtype一致。

@zhiboniu
Copy link
Contributor

你可以参照一下行业内其他框架做法,一般都是行业标准。这里的randint应该只是随机值是整数,并不影响其dtype类型,所以dtype应该也可以是float的。
应该同时满足你的2、3条件。

如果有不清楚的地方,一般可以多去看看其他框架,了解一下行业标准。

@JunnYu
Copy link
Member Author

JunnYu commented Oct 19, 2021

好的,之后我改一下

@JunnYu JunnYu requested a review from zhiboniu October 23, 2021 15:31
@JunnYu JunnYu requested a review from zhiboniu October 25, 2021 07:32
@JunnYu JunnYu requested a review from zhiboniu October 26, 2021 12:13
Copy link
Contributor

@zhiboniu zhiboniu left a comment

Choose a reason for hiding this comment

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

lgtm

@JunnYu
Copy link
Member Author

JunnYu commented Oct 31, 2021

中文文档已提交PR: PaddlePaddle/docs#4028

Copy link
Contributor

@TCChenlong TCChenlong left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LG API

tools/parallel_UT_rule.py Show resolved Hide resolved
Copy link
Contributor

@jeff41404 jeff41404 left a comment

Choose a reason for hiding this comment

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

lgtm

@jeff41404 jeff41404 merged commit 41a0911 into PaddlePaddle:develop Nov 2, 2021
@JunnYu JunnYu deleted the add_randint_like branch November 2, 2021 10:08
ghost pushed a commit to piotrekobi/Paddle that referenced this pull request Nov 3, 2021
* add randint like

* rm .cc .cu

* Update unity_build_rule.cmake

* try to make test pass

* use python

* update

* update randint_like

* rename test_randint_like_op -> test_randint_like

* update

* update randint like docs

* update randint like

* update

* update

* add bool

* update randint like test

* update

* update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants