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

【Hackathon 4 No.21】Add i1 / i1e to paddle #53210

Merged
merged 2 commits into from
May 17, 2023

Conversation

LyndonKong
Copy link
Contributor

@LyndonKong LyndonKong commented Apr 22, 2023

PR types

New features

PR changes

APIs

Description

We add i1 and i1e ops for paddle.
document link is here
rfc link is here

@paddle-bot
Copy link

paddle-bot bot commented Apr 22, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Apr 22, 2023
@paddle-bot
Copy link

paddle-bot bot commented Apr 22, 2023

❌ The PR is not created using PR's template. You can refer to this Demo.
Please use PR's template, it helps save our maintainers' time so that more developers get helped.

@LyndonKong
Copy link
Contributor Author

We are working on to fix the bugs in backward operators, not ready to be reviewed.

@LyndonKong
Copy link
Contributor Author

LyndonKong commented Apr 25, 2023

Done with our local unittests, this PR is now ready to be reviewed @luotao1 .

@LyndonKong
Copy link
Contributor Author

Backward bugs on GPU, try to test GPU in our local machine.

@zhengqiwen1997
Copy link
Contributor

我们内部在审核i0/i0e的PR后,认为使用Eigen可能不支持后续高维度Tensor的开展,具体原因请见PR下方我的评论。所以需要麻烦你修改一下实现策略,可以考虑之前i0/i0e的实现方法,并且需要先修改rfc文档。为你的开发过程带来的不便深感抱歉!

@LyndonKong
Copy link
Contributor Author

我们内部在审核i0/i0e的PR后,认为使用Eigen可能不支持后续高维度Tensor的开展,具体原因请见PR下方我的评论。所以需要麻烦你修改一下实现策略,可以考虑之前i0/i0e的实现方法,并且需要先修改rfc文档。为你的开发过程带来的不便深感抱歉!

OK, we will change our implementation to this version and update our RFC document in a few days.

@LyndonKong LyndonKong force-pushed the bessel_func branch 8 times, most recently from ffd5e53 to 96fb135 Compare May 5, 2023 13:44
@LyndonKong
Copy link
Contributor Author

This pr has passed all CI checks, we would appreciate if you could review it @luotao1 @zhengqiwen1997 .

zhengqiwen1997
zhengqiwen1997 previously approved these changes May 9, 2023
@LyndonKong
Copy link
Contributor Author

辛苦review @luotao1

@luotao1
Copy link
Contributor

luotao1 commented May 10, 2023

@LyndonKong 待 i0/i0e 技术 approve 后,会一起进入下一轮审核

zhengqiwen1997
zhengqiwen1997 previously approved these changes May 12, 2023
Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

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

Returns:
- out (Tensor), A Tensor. the value of the modified bessel function of order 1 at x.
Examples:
.. code-block:: python
Copy link
Contributor

Choose a reason for hiding this comment

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

在code-block下方空行,否则解析错误,参考paddle.add的写法

    Examples:
        .. code-block:: python

            import paddle

            x = paddle.to_tensor([2, 3, 4], 'float64')
            y = paddle.to_tensor([1, 5, 2], 'float64')
            z = paddle.add(x, y)
            print(z)  # [3., 8., 6. ]

Returns:
- out (Tensor), A Tensor. the value of the exponentially scaled modified Bessel function of order 1 at x.
Examples:
.. code-block:: python
Copy link
Contributor

Choose a reason for hiding this comment

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

同样的错误,需要加空行

def i1(x, name=None):
"""
The function is used to calculate modified bessel function of order 1.
Args:
Copy link
Contributor

Choose a reason for hiding this comment

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

args上方加空行,否则解析错误

Args:
x (Tensor): The input tensor, it's data type should be float32, float64.
name (str, optional): For details, please refer to :ref:`api_guide_Name`. Generally, no setting is required. Default: None.
Returns:
Copy link
Contributor

Choose a reason for hiding this comment

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

returns上方加空行

name (str, optional): For details, please refer to :ref:`api_guide_Name`. Generally, no setting is required. Default: None.
Returns:
- out (Tensor), A Tensor. the value of the modified bessel function of order 1 at x.
Examples:
Copy link
Contributor

Choose a reason for hiding this comment

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

examples上方加空行

def i1e(x, name=None):
"""
The function is used to calculate exponentially scaled modified Bessel function of order 1.
Args:
Copy link
Contributor

Choose a reason for hiding this comment

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

上方加空行

Args:
x (Tensor): The input tensor, it's data type should be float32, float64.
name (str, optional): For details, please refer to :ref:`api_guide_Name`. Generally, no setting is required. Default: None.
Returns:
Copy link
Contributor

Choose a reason for hiding this comment

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

上方加空行

name (str, optional): For details, please refer to :ref:`api_guide_Name`. Generally, no setting is required. Default: None.
Returns:
- out (Tensor), A Tensor. the value of the exponentially scaled modified Bessel function of order 1 at x.
Examples:
Copy link
Contributor

Choose a reason for hiding this comment

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

上方加空行

@LyndonKong
Copy link
Contributor Author

Fix the mentioned issue of doc-string in our new commit @sunzhongkai588

Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

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

LGTM for docs

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

@luotao1 luotao1 merged commit a63fb4c into PaddlePaddle:develop May 17, 2023
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants