Skip to content

Conversation

@Difers
Copy link
Contributor

@Difers Difers commented Aug 11, 2025

PR Category

Operator Mechanism

PR Types

Improvements

Description

Pcard-73145

根据comment:#74545 (comment)

  • 检测api传入paddle.dtype支持情况,针对各种报错case进行了处理
  • 多数错误很可能是convert_np_dtype_to_dtype_传入了paddle.dtype导致错误(之前都是VarType,因此一些算子并没有对coreType的处理),因此对convert_np_dtype_to_dtype_做了增强避免出现没有检测到的问题

@paddle-bot
Copy link

paddle-bot bot commented Aug 11, 2025

你的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.

Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

看大部分只修改了文档,这些已经验证过了 paddle.dtype都是支持的对吧?

@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2025

Codecov Report

❌ Patch coverage is 86.36364% with 6 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (develop@69caf6a). Learn more about missing BASE report.

Files with missing lines Patch % Lines
python/paddle/base/framework.py 83.33% 2 Missing ⚠️
...thon/paddle/incubate/nn/functional/int_bincount.py 33.33% 2 Missing ⚠️
python/paddle/tensor/math.py 87.50% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             develop   #74545   +/-   ##
==========================================
  Coverage           ?   86.36%           
==========================================
  Files              ?        8           
  Lines              ?       44           
  Branches           ?        0           
==========================================
  Hits               ?       38           
  Misses             ?        6           
  Partials           ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Difers
Copy link
Contributor Author

Difers commented Aug 12, 2025

看大部分只修改了文档,这些已经验证过了 paddle.dtype都是支持的对吧?

一般来说应该不会有paddle.dtype不支持的情况,唯一的可能情况是在静态图模式传入了paddle.dtype,或者动态图传入VarType;我可以再写个脚本检测一下这两种情况完善一下~

@zhwesky2010
Copy link
Contributor

看大部分只修改了文档,这些已经验证过了 paddle.dtype都是支持的对吧?

一般来说应该不会有paddle.dtype不支持的情况,唯一的可能情况是在静态图模式传入了paddle.dtype,或者动态图传入VarType;我可以再写个脚本检测一下这两种情况完善一下~

有一个低成本的测试方式,在这个里面把所有的 'float32' 都改成 paddle.float32 这种,然后提交下CI看单测都能不能过,之前是发现无法通过,才只能将torch.dtype映射到字符串,这个工作完成了按道理 就应该将 torch.dtype映射到paddle.dtype

https://github.com/PaddlePaddle/PaConvert/blob/master/paconvert/attribute_mapping.json#L171

@Difers
Copy link
Contributor Author

Difers commented Aug 13, 2025

看大部分只修改了文档,这些已经验证过了 paddle.dtype都是支持的对吧?

一般来说应该不会有paddle.dtype不支持的情况,唯一的可能情况是在静态图模式传入了paddle.dtype,或者动态图传入VarType;我可以再写个脚本检测一下这两种情况完善一下~

有一个低成本的测试方式,在这个里面把所有的 'float32' 都改成 paddle.float32 这种,然后提交下CI看单测都能不能过,之前是发现无法通过,才只能将torch.dtype映射到字符串,这个工作完成了按道理 就应该将 torch.dtype映射到paddle.dtype

https://github.com/PaddlePaddle/PaConvert/blob/master/paconvert/attribute_mapping.json#L171

已根据该方案进行了本地测试与修改,等本PR合入后,再看一下PaConvert单测是否能全部通过,如有遗漏,后续会再提PR补充;另,相关api注释涉及较多,也后续另提pr修改,方便查看。

Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

那dtype文档后面再统一修改吧,这个也挺乱的


# check amp_dtype: float16 or bfloat16
dtype = dtype.lower()
if isinstance(dtype, str):
Copy link
Contributor

Choose a reason for hiding this comment

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

这些逻辑可以写一个公共函数,将所有输入的dtype归一化成str类型

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改~

"""
if not isinstance(dtype, core.VarDesc.VarType):
if dtype is None:
dtype = paddle.get_default_dtype()
Copy link
Contributor

@zhwesky2010 zhwesky2010 Aug 13, 2025

Choose a reason for hiding this comment

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

get_default_dtype仅针对浮点型的,这个是int32或64

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改~

@Difers
Copy link
Contributor Author

Difers commented Aug 14, 2025

/re-run all-failed

zhwesky2010
zhwesky2010 previously approved these changes Aug 14, 2025
Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhwesky2010
Copy link
Contributor

infoflow 2025-08-14 19-56-31

单测这里挂了

@zhwesky2010
Copy link
Contributor

API benchmark没过,rerun再试试

@Difers
Copy link
Contributor Author

Difers commented Aug 19, 2025

/re-run all-failed

@zhwesky2010
Copy link
Contributor

@Difers API-benchmark还是没过,应该新增加的python判断分支导致的,看看怎么精简一下python判断逻辑

infoflow 2025-08-19 19-31-50

@Difers Difers force-pushed the add_dtype_check branch 3 times, most recently from b6f3ab1 to 417bdd2 Compare August 21, 2025 05:48
@cyy536 cyy536 mentioned this pull request Aug 22, 2025
@Difers Difers force-pushed the add_dtype_check branch 2 times, most recently from 4d55f93 to 9be6d21 Compare August 25, 2025 02:31
@Difers Difers force-pushed the add_dtype_check branch 2 times, most recently from 1c603b7 to f55116b Compare August 25, 2025 05:22
"""
if use_pir_api():
if isinstance(np_dtype, core.DataType):
Copy link
Contributor

Choose a reason for hiding this comment

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

这里的判断是不是就不需要了,一般会在外面判断了

if isinstance(np_dtype, (core.DataType, core.VarDesc.VarType)):
      ...


if isinstance(np_dtype, core.VarDesc.VarType):
return np_dtype
return convert_np_dtype_to_proto_type(np_dtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

convert_np_dtype_to_proto_type这个函数的这些内容,可以换成map的实现,提升性能:

infoflow 2025-08-25 15-39-07

同时可以直接 str映射到core.VarDesc.VarType,无需先转np.dtype,再转core.VarDesc.VarType

dtype = x.dtype
if not isinstance(dtype, (core.VarDesc.VarType, core.DataType)):
dtype = convert_np_dtype_to_dtype_(dtype)
if dtype is not None and x.dtype != dtype:
Copy link
Contributor

Choose a reason for hiding this comment

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

这里dtype还有None的情况吗,会不会导致多判断了一次

dtype = x.dtype
if not isinstance(dtype, (core.VarDesc.VarType, core.DataType)):
dtype = convert_np_dtype_to_dtype_(dtype)
if dtype is not None and x.dtype != dtype:
Copy link
Contributor

Choose a reason for hiding this comment

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

同上


if in_dynamic_or_pir_mode():
if dtype is not None:
if dtype is not None and x.dtype != dtype:
Copy link
Contributor

Choose a reason for hiding this comment

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

这里dtype没有None的情况

dtype = x.dtype
if not isinstance(dtype, (core.VarDesc.VarType, core.DataType)):
dtype = convert_np_dtype_to_dtype_(dtype)
if dtype is not None and x.dtype != dtype:
Copy link
Contributor

Choose a reason for hiding this comment

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

这里dtype没有None的情况

x = cast(x, dtype)
if dtype is None and paddle.is_tensor(x):
dtype = x.dtype
if dtype is not None and x.dtype != dtype:
Copy link
Contributor

Choose a reason for hiding this comment

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

这里dtype没有None的情况

@zhwesky2010
Copy link
Contributor

需要再优化一下上面的分支逻辑判断,提升下性能。

@Difers Difers force-pushed the add_dtype_check branch 5 times, most recently from 39cc60d to c7e40bd Compare August 27, 2025 05:46
@Difers
Copy link
Contributor Author

Difers commented Aug 27, 2025

/re-run all-failed

zhwesky2010
zhwesky2010 previously approved these changes Aug 27, 2025
Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

LGTM

@Difers
Copy link
Contributor Author

Difers commented Aug 28, 2025

/re-run all-failed

2 similar comments
@Difers
Copy link
Contributor Author

Difers commented Aug 28, 2025

/re-run all-failed

@Difers
Copy link
Contributor Author

Difers commented Aug 28, 2025

/re-run all-failed

Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhwesky2010 zhwesky2010 merged commit 917f172 into PaddlePaddle:develop Aug 28, 2025
112 of 119 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants