8000 [inductor] align `replicationpad` on processing `bool` dtype with eager by shaoyuyoung · Pull Request #147666 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

[inductor] align replicationpad on processing bool dtype with eager #147666

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

[inductor] align replicationpad on processing bool dtype with eager #147666

wants to merge 8 commits into from

Conversation

shaoyuyoung
Copy link
Contributor
@shaoyuyoung shaoyuyoung commented Feb 22, 2025 •
edited by pytorch-bot bot
Loading

@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Feb 22, 2025 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit d68a455 with merge base dda0c95 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@shaoyuyoung
10000 Copy link
Contributor Author

@pytorchbot label "topic: not user facing"

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Feb 22, 2025
zeshengzong
zeshengzong reviewed Feb 22, 2025
View reviewed changes
@soulitzer soulitzer requested review from jansel and eellison February 24, 2025 16:34
@soulitzer soulitzer added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Feb 24, 2025
eellison
eellison reviewed Feb 24, 2025
View reviewed changes
Copy link
Contributor
@eellison eellison left a comment

Choose a reason for hiding this comment

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

Some of the operators just did not have cuda kernels written for them in long tail dtypes. If torch.compile is correctly computing the semantics do we need to make this a hard error ?

You could add a test of replication pad with bool inputs compared to the same inputs converted to ints and run through eager to check.

cc @zou3519 for other thoughts

@zou3519
Copy link
Contributor
zou3519 commented Feb 24, 2025

I can see this going both ways:

  • Thought process 1: torch.compile is supposed to match eager semantics, so it should emulate everything that eager does...
  • Thought process 2: all operators should support all dtypes, it's eager mode's problem that it doesn't support the additional dtypes, we should just expand the eager dtypes instead of disabling the compile ones

@shaoyuyoung shaoyuyoung mentioned this pull request Mar 1, 2025
Open
@shaoyuyoung
Copy link
Contributor Author

have addressed the lint error

@shaoyuyoung shaoyuyoung requested a review from jansel April 25, 2025 04:34
@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Apr 26, 2025

Pull workflow has not been scheduled for the PR yet. It could be because author doesn't have permissions to run those or skip-checks keywords were added to PR/commits, aborting merge. Please get/give approval for the workflows and/or remove skip ci decorators before next merge attempt. If you think this is a mistake, please contact PyTorch Dev Infra.

@shaoyuyoung
Copy link
Contributor Author

Anyone can help me trigger the merge?
Thanks

@jansel
Copy link
Contributor
jansel commented Apr 26, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 26, 2025
@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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@pytorch-bot pytorch-bot bot removed the ciflow/trunk Trigger trunk jobs on your pull request label Apr 27, 2025
@jansel
Copy link
Contributor
jansel commented Apr 27, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 27, 2025
@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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers
8000

@pytorch-bot pytorch-bot bot removed the ciflow/trunk Trigger trunk jobs on your pull request label Apr 28, 2025
@jansel
Copy link
Contributor
jansel commented Apr 28, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 28, 2025
@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eellison eellison eellison left review comments

@zeshengzong zeshengzong zeshengzong left review comments

@jansel jansel jansel approved these changes

Assignees
No one assigned
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged module: inductor open source topic: not user facing topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[inductor] [dtype] ReplicationPad raise dtype error on eager but pass the check on indcutor
8 participants
@shaoyuyoung @zou3519 @jansel @pytorchmergebot @eellison @zeshengzong @soulitzer @pytorchbot
0