Skip to content

Conversation

@SigureMo
Copy link
Member

@SigureMo SigureMo commented Jun 9, 2025

PR Category

Execute Infrastructure

PR Types

Bug fixes

Description

在此之前,AST 动转静在根据输入计算 hash 时会依次对输入进行 hash,而 dataclass 本身是 unhashable 的,因此需要单独写一下 hash 的方式

当然,SOT 是没有这个问题的,但 SOT 有其他的问题(无法识别内部数据,会 hold 内部的 Tensor,有 OOM 的隐患)

@SigureMo SigureMo requested review from gouzil and zrr1999 as code owners June 9, 2025 12:27
@paddle-bot
Copy link

paddle-bot bot commented Jun 9, 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.

@SigureMo SigureMo requested review from Copilot and removed request for gouzil and zrr1999 June 9, 2025 12:27
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for using Python dataclass instances as inputs to Paddle’s dynamic-to-static (dy2static) API by implementing custom hashing logic and verifying it with a new unit test.

  • Introduce _is_dataclass_instance and extend make_hashable to handle dataclass objects.
  • Add import of is_dataclass from the standard library.
  • Add a new test to verify that a dataclass wrapping a Tensor can be passed through paddle.jit.to_static.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
test/dygraph_to_static/test_dataclass_as_input.py New unit test to ensure dataclass input is accepted and matches output
python/paddle/jit/dy2static/utils.py Import is_dataclass; add detection and hashing logic for dataclass
Comment on lines 307 to 314
return tuple(
map(
make_hashable,
[
getattr(x, field_name)
for field_name in x.__dataclass_fields__
],
)
Copy link

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

Two different dataclass types with identical field values will produce the same hash, which may cause unintended cache collisions. Consider including the dataclass’s type (e.g. type(x) or its qualified name) as part of the returned tuple to distinguish between types.

Suggested change
return tuple(
map(
make_hashable,
[
getattr(x, field_name)
for field_name in x.__dataclass_fields__
],
)
return (
type(x).__name__,
tuple(
map(
make_hashable,
[
getattr(x, field_name)
for field_name in x.__dataclass_fields__
],
)
),
Copilot uses AI. Check for mistakes.
make_hashable,
[
getattr(x, field_name)
for field_name in x.__dataclass_fields__
Copy link

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

Relying on the internal __dataclass_fields__ dict risks future incompatibilities and implicitly depends on dict ordering. Use dataclasses.fields(x) to get a stable, documented list of fields in definition order.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (develop@d24f6a4). Learn more about missing BASE report.

Additional details and impacted files
@@ Coverage Diff @@ ## develop #73205 +/- ## =========================================== Coverage ? 100.00% =========================================== Files ? 1 Lines ? 5 Branches ? 0 =========================================== Hits ? 5 Misses ? 0 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.
@SigureMo SigureMo merged commit 917ecbd into PaddlePaddle:develop Jun 10, 2025
70 of 73 checks passed
@SigureMo SigureMo deleted the dy2st/allow-dataclass-as-input-argument branch June 10, 2025 14:40
shanjiang7 pushed a commit to shanjiang7/Paddle that referenced this pull request Jun 12, 2025
--------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants