8000 Refactor instance_descriptor for new triton version by bhack · Pull Request #119636 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

Refactor instance_descriptor for new triton version #119636

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 5 commits into from
Closed

Refactor instance_descriptor for new triton version #119636

wants to merge 5 commits into from

Conversation

bhack
Copy link
Contributor
@bhack bhack commented Feb 10, 2024 •
edited by pytorch-bot bot
Loading

@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Feb 10, 2024 •
edited
Loading

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 7b720a6 with merge base 3629287 (image):

NEW FAILURE - The following job has failed:

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

@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Feb 10, 2024

Please seek CI approval before scheduling CIFlow labels

@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Feb 10, 2024

Please seek CI approval before scheduling CIFlow labels

@bhack bhack mentioned this pull request Feb 10, 2024
Closed
@bhack
Copy link
Contributor Author
bhack commented Feb 10, 2024

/cc @shunting314

Honestly I see this approach really risk if we don't have the resources to test in CI with all the Trition version we want to be compatible.
So IMHO it is better to enforce our specific Trition version given a pytorch commit release but that at least it is really tested in the specific trition version.

E.g who is still going to test the case except ImportError: about the regression? I don't think that the CI is going to cover this.

@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Feb 10, 2024

Please seek CI approval before scheduling CIFlow labels

Skylion007
Skylion007 reviewed Feb 10, 2024
View reviewed changes
@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Feb 11, 2024

Please seek CI approval before scheduling CIFlow labels

@bhack bhack requested a review from Skylion007 February 11, 2024 16:56
@bhack
Copy link
Contributor Author
bhack commented Feb 12, 2024

@peterbell10 Can you re-label?

@bhack bhack mentioned this pull request Feb 12, 2024
Closed
@bhack

This comment was marked as outdated.

Sign in to view
@bhack

This comment was marked as outdated.

Sign in to view
@bhack

This comment was marked as outdated.

Sign in to view
@bhack
Copy link
Contributor Author
bhack commented Feb 12, 2024

@Skylion007 what do you think?

@bhack
Copy link
Contributor Author
bhack commented Feb 13, 2024 •
edited
Loading

I've changed a little bit the logic, I've preferred the previous one, but I don't know if in the codegen we could inject a dataclass or any other module import.

shunting314
shunting314 reviewed Feb 13, 2024
View reviewed changes
shunting314
shunting314 reviewed Feb 13, 2024
View reviewed changes
shunting314
shunting314 reviewed Feb 14, 2024
View reviewed changes
@bhack bhack requested a review from shunting314 February 15, 2024 02:32
@bhack
Copy link
Contributor Author
bhack commented Feb 15, 2024

/cc @bertmaher

shunting314
shunting314 approved these changes Feb 15, 2024
View reviewed changes
@bhack bhack requested a review from shunting314 February 15, 2024 18:39
@kit1980 kit1980 changed the title Refactor instance_descriptor for new trition version Refactor instance_descriptor for new triton version Feb 16, 2024
@bhack
Copy link
Contributor Author
bhack commented Feb 20, 2024

is this ok now?

@bhack bhack requested a review from shunting314 February 20, 2024 12:07
@bhack
Copy link
Contributor Author
bhack commented Feb 20, 2024

I've fixed the residual linting

@bhack
Copy link
Contributor Author
bhack commented Feb 20, 2024 •
edited
Loading

@shunting314 That single failure is unrelated to the PR

@shunting314
Copy link
Contributor
shunting314 commented Feb 20, 2024 •
edited
Loading

That single failure is unrelated to the PR?

Looks correct.

@bhack
Copy link
Contributor Author
bhack commented Feb 20, 2024

So what we want to do?

@peterbell10
Copy link
Collaborator

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 1 checks: pull / linux-focal-py3.12-clang10 / build

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

@peterbell10
Copy link
Collaborator

@pytorchbot merge -f "Unrelated CI failure"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

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

@shunting314 shunting314 shunting314 approved these changes

@Skylion007 Skylion007 Awaiting requested review from Skylion007

Assignees
No one assigned
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: inductor open source topic: not user facing topic category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bhack @shunting314 @peterbell10 @pytorchmergebot @Skylion007 @pytorchbot
0