Skip to content

Conversation

pianpwk
Copy link
Contributor

@pianpwk pianpwk commented May 13, 2025

Fixes #138853

Summary: Converts TruncToInt(IntTrueDiv(x / y)) to x // y if divisible, helps detect symint specializations where we didn't previously

Differential Revision: D74664734

cc @ezyang @SherlockNoMad @EikanWang @jgong5 @wenzhe-nrv

Copy link

pytorch-bot bot commented May 13, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/153477

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 268e407 with merge base 82dc345 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74664734

r"your code specialized it to be a constant \(28\)(.*\n)*.*"
),
):
export(model, input1, dynamic_shapes=dynamic_shapes, strict=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't investigate more, but strict=True doesn't emit this guard; the original test case passes

@pianpwk pianpwk requested review from angelayi and avikchaudhuri May 14, 2025 16:36
@bobrenjc93
Copy link
Contributor

Hmm are you sure this is sound?

>>> int(-3 / 2)
-1
>>> -3 // 2
-2

Do you need to limit to only positive integers?

@pianpwk
Copy link
Contributor Author

pianpwk commented May 15, 2025

Hmm are you sure this is sound?

>>> int(-3 / 2)
-1
>>> -3 // 2
-2

it's only for divisible terms, so I don't think we'd apply this here?

Do you need to limit to only positive integers?

I don't think it's limited, unless I'm missing something...

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label May 15, 2025
@pianpwk
Copy link
Contributor Author

pianpwk commented May 15, 2025

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Summary: detects symint specializations where we didn't previously

Differential Revision: D74664734
@pytorchmergebot
Copy link
Collaborator

Successfully rebased export-D74664734 onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout export-D74664734 && git pull --rebase)

@pianpwk
Copy link
Contributor Author

pianpwk commented May 15, 2025

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR has internal changes and must be landed via Phabricator! Please try reimporting/rexporting the PR!

Details for Dev Infra team Raised by workflow job

@facebook-github-bot
Copy link
Contributor

@pianpwk has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@github-actions github-actions bot deleted the export-D74664734 branch June 19, 2025 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request fb-exported fx Merged release notes: fx release notes category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using int(shape) in export would result in silent specialization

4 participants