| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
【Hackathon 6th No.8】NO.8 为 Paddle 新增 FeatureAlphaDropout API #64881
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
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
megemini
changed the title
|
我在本地测试了一下， =========================================================================== short test summary info ============================================================================
FAILED test_dropout_op.py::TestDropoutOp2::test_check_output - RuntimeError: (PreconditionNotMet) [Prim] For op pd_op.dropout, its origin 1-index output rank of shape[32, 64] is not equal to decomp output rank of shape[] (at /home/sh...
FAILED test_dropout_op.py::TestDropoutOp6::test_check_output - RuntimeError: (PreconditionNotMet) [Prim] For op pd_op.dropout, its origin 1-index output rank of shape[32, 64] is not equal to decomp output rank of shape[] (at /home/sh...
FAILED test_dropout_op.py::TestBF16DropoutOp::test_check_grad_normal - RuntimeError: (PreconditionNotMet) [Prim] For op pd_op.dropout, its origin 1-index output rank of shape[32, 64] is not equal to decomp output rank of shape[] (at /home/sh...
FAILED test_dropout_op.py::TestBF16DropoutOp::test_check_output - RuntimeError: (PreconditionNotMet) [Prim] For op pd_op.dropout, its origin 1-index output rank of shape[32, 64] is not equal to decomp output rank of shape[] (at /home/sh...
FAILED test_dropout_op.py::TestPirCompositeDropout_6_p_1_0_test_False::test_static_comp - RuntimeError: (PreconditionNotMet) [Prim] For op pd_op.dropout, its origin 1-index output rank of shape[100000] is not equal to decomp output rank of shape[] (at /home/sh...
FAILED test_dropout_op.py::TestPirCompositeDropout_7_p_1_0_test_False_dtype_bfp16::test_static_comp - RuntimeError: (PreconditionNotMet) [Prim] For op pd_op.dropout, its origin 1-index output rank of shape[100000] is not equal to decomp output rank of shape[] (at /home/sh...另外，我看 CI 里面貌似没有测试这个 还是说，我把 alpha dropout 和 feature alpha dropout 的测试用例单独拎一个文件出来测试？ 还请帮忙看看 ～ 谢谢！ |
| @@ -12,6 +12,8 @@ | |||
| # See the License for the specific language governing permissions and | |||
| # limitations under the License. | |||
|
|
|||
| from __future__ import annotations | |||
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.
这是干什么的？
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.
目前在做类型标注 #65008
所以这里索性直接加上了 ～
| x (Tensor): The input tensor. The data type is bfloat16, float16, float32 or float64. | ||
| p (float | int, optional): Probability of setting units to zero. Default 0.5. | ||
| training (bool, optional): A flag indicating whether it is in train phrase or not. Default True. | ||
| name (str | None, optional): Name for the operation (optional, default is None). For more information, please refer to :ref:`api_guide_Name`. |
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.
不需要|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.
这里我解释一下哈 ～ 按理说 type hints 做好之后，docstring 里面的 Args 不需要再写类型了，比如：
原来是
def foo(name=None):
"""
...
Args:
name (str, optional): xxx
"""
...应该是
def foo(name: str | None = None) -> None:
"""
...
Args:
name : xxx
"""
...但是，由于 paddle 之前没做 type hints，所以保留了 Args 里面的参数类型：
def foo(name: str | None = None) -> None:
"""
...
Args:
name (str | None, optional) : xxx
"""
...这里的 optional 与 python 的 Optional 类型不一样，docstring 里面的 optional 表示有默认值，None 也算，所以这里 str | None, optional 与 str, optional 是不一样的：
str | None, optional表示，此参数是 str 类型，且有默认值，默认值为 Nonestr, optional表示，此参数是 str 类型，且有默认值，但是无法表示默认值是什么
python/paddle/nn/layer/common.py
Outdated
| p (float | int): Probability of setting units to zero. Default: 0.5 | ||
| name (str, optional): Name for the operation (optional, default is None). For more information, please refer to :ref:`api_guide_Name`. | ||
| p (float | int, optional): Probability of setting units to zero. Default: 0.5 | ||
| name (str | None, optional): Name for the operation (optional, default is None). For more information, please refer to :ref:`api_guide_Name`. |
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.
同上
|
|
||
| Parameters: | ||
| p (float | int, optional): Probability of setting units to zero. Default: 0.5 | ||
| name (str | None, optional): Name for the operation (optional, default is None). For more information, please refer to :ref:`api_guide_Name`. |
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.
同上
test/legacy_test/test_dropout_op.py
Outdated
|
|
||
| res_list = [res1, res2] | ||
| for res in res_list: | ||
| np.testing.assert_allclose(res.numpy(), res_np, rtol=1e-05) |
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.
也需要检测一下梯度和反向
test/legacy_test/test_dropout_op.py
Outdated
| self.assertRaises(TypeError, test_Variable) | ||
|
|
||
| @test_with_pir_api | ||
| def test_errors2(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.
给出有意义的命名
test/legacy_test/test_dropout_op.py
Outdated
|
|
||
| class TestFeatureAlphaDropoutFAPIError(unittest.TestCase): | ||
| @test_with_pir_api | ||
| def test_errors(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.
给出有意义的命名
test/legacy_test/test_dropout_op.py
Outdated
| @@ -1319,6 +1319,168 @@ def test_static_fp16_gpu(self): | |||
| np.testing.assert_allclose(res[0], input, rtol=1e-05) | |||
|
|
|||
|
|
|||
| class TestFeatureAlphaDropoutFAPI(unittest.TestCase): | |||
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.
TestFeatureAlphaDropoutFAPI中的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.
我是参考此测试文件中其他用例写的，好像是这里的习惯写法？ F 应该是值 function，也就是 feature_alpha_dropout ，C 应该是指 class，也就是 FeatureAlphaDropout ～
那我改一下吧 ～
test/legacy_test/test_dropout_op.py
Outdated
| self.assertRaises(ValueError, test_pvalue) | ||
|
|
||
|
|
||
| class TestFeatureAlphaDropoutCAPI(unittest.TestCase): |
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.
TestFeatureAlphaDropoutCAPI中的C是指什么？另外建议增加bfloat16的测试
目前看这个属于存量api的问题，你可以单开一个单测。 |
嗯好！那我把 alpha dropout 和 feature alpha dropout 单独拎一个测试文件！ |
python/paddle/nn/layer/common.py
Outdated
| [-1., 1.]]) | ||
| """ | ||
|
|
||
| def __init__(self, p=0.5, 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.
这个 API 忘记标注了？
python/paddle/nn/layer/common.py
Outdated
| ) | ||
| return out | ||
|
|
||
| def extra_repr(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.
| def extra_repr(self): | |
| def extra_repr(self) -> str: |
megemini
changed the title
megemini
changed the title
megemini
changed the title
megemini
changed the title
python/paddle/_typing/__init__.py
Outdated
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 里做了这么多其他的类型提示改动？这会影响这个 PR 本身的 review，不建议这样做
PR 应拆尽拆，不要什么都放在一个大 PR 里
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.
建议将刚刚的类型提示 commit cherry-pick 到新的分支，并在本分支 revert 掉相关 commit
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.
收到 ～ 😁
This reverts commit b79f264.
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.
LGTMeow 
SigureMo
changed the title
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
|
Sorry to inform you that c042e1e'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 for docs
PR Category
User Experience
PR Types
New features
Description
NO.8 为 Paddle 新增 FeatureAlphaDropout API
相关接口的实现代码。
相关 RFC PaddlePaddle/community#913
20240604