Skip to content

Conversation

@yf225
Copy link
Contributor

@yf225 yf225 commented Nov 5, 2025

Fixes #1067.

@yf225 yf225 requested review from jansel, oulgen and v0i0 November 5, 2025 00:38
@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 5, 2025
@yf225 yf225 force-pushed the fix_gated_delta_net_1067_issue branch from 2956740 to 1a718f2 Compare November 5, 2025 00:38
@yf225 yf225 marked this pull request as draft November 5, 2025 00:46
@yf225 yf225 force-pushed the fix_gated_delta_net_1067_issue branch 23 times, most recently from 7c96893 to 0779ba6 Compare November 5, 2025 22:24
@yf225 yf225 force-pushed the fix_gated_delta_net_1067_issue branch 2 times, most recently from d620f11 to af6c723 Compare November 5, 2025 22:52
@yf225 yf225 force-pushed the fix_gated_delta_net_1067_issue branch from af6c723 to 39c206d Compare November 5, 2025 23:01
@yf225 yf225 marked this pull request as ready for review November 5, 2025 23:46
)
ast.parse(code)

@skipIfPyTorchBaseVerLessThan("2.10")
Copy link
Contributor Author

@yf225 yf225 Nov 5, 2025

Choose a reason for hiding this comment

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

PyTorch 2.9 has a indexing expression related bug that's only fixed in PyTorch nightly: pytorch/pytorch#131761. Making a patch for that in Helion framework is quite complicated, thus we skip this unit test unless we are on 2.10 version (including current nightly).

if isinstance(x, int):
return sympy.Integer(x)
if isinstance(x, sympy.Expr):
return sympy.simplify(x)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a need to simplify here? We don't simplify if given a symint. (Note sympify!=simplify)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed we don't have to simplify here (was copying from existing resolve_block_id code, but removing it also passes tests)

@yf225 yf225 merged commit f05043e into main Nov 7, 2025
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gated delta net kernel correctness issue

3 participants