Skip to content
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.

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

nn.Linear with BSR inputs: spare the user from explicit Triton kernel registrations #98403

Closed
wants to merge 22 commits into from

Conversation

nikitaved
Copy link
Collaborator

@nikitaved nikitaved commented Apr 5, 2023

Stack from ghstack (oldest at bottom):

🤖 Generated by Copilot at 08f7a6a

This pull request adds support for triton kernels in torch and torch/cuda, and refactors and tests the existing triton kernel for BSR matrix multiplication. It also adds a test case to ensure that importing torch does not implicitly import triton.

cc @alexsamardzic @pearu @cpuhrsch @amjames @bhosmer @albanD @mruberry @jbschlosser @walterddr @mikaylagawarecki

@pytorch-bot
Copy link

pytorch-bot bot commented Apr 5, 2023

🔗 Helpful Links

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

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

✅ No Failures

As of commit cec539a:
💚 Looks good so far! There are no failures yet. 💚

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

nikitaved added a commit that referenced this pull request Apr 5, 2023
… registrations

ghstack-source-id: 65ede8fe957956a0ea12d5ff5bfb2a68e3ae4e1a
Pull Request resolved: #98403
@nikitaved nikitaved added module: sparse Related to torch.sparse module: nn Related to torch.nn ciflow/trunk Trigger trunk jobs on your pull request release notes: sparse release notes category labels Apr 5, 2023
…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request Apr 5, 2023
… registrations

ghstack-source-id: 313c3c181d2722b3727d5cdc1ff0e2655d1d4f0b
Pull Request resolved: #98403
@nikitaved nikitaved marked this pull request as draft April 5, 2023 13:29
…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request Apr 5, 2023
… registrations

ghstack-source-id: 16882701efe7e7b3db31e39c2ecf5f4d4a2c3116
Pull Request resolved: #98403
@nikitaved nikitaved requested a review from malfet April 5, 2023 16:13
@nikitaved nikitaved marked this pull request as ready for review April 6, 2023 08:54
…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request Apr 6, 2023
… registrations

ghstack-source-id: d5c2b4a21c97a7cec8257cfd17333bd240aa61fc
Pull Request resolved: #98403
…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request Apr 12, 2023
… registrations

ghstack-source-id: 31d1ee601444da87c953688184f5aeb97bdd7dbc
Pull Request resolved: #98403
…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request Apr 14, 2023
… registrations

ghstack-source-id: d073b22acdbcb0d651efeeb1a45b4ffa129d66e7
Pull Request resolved: #98403
…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request Apr 18, 2023
… registrations

ghstack-source-id: 5eaddc32ff0f5337d6e246c1fcc25e51622b6c65
Pull Request resolved: #98403
@cpuhrsch cpuhrsch requested a review from ezyang April 18, 2023 14:36
@cpuhrsch
Copy link
Contributor

@ezyang @malfet - Can you take a look at this?

…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request May 25, 2023
… registrations

ghstack-source-id: 75b2b0000bbef2dc5b78359e112656867ce95d0e
Pull Request resolved: #98403
…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request May 25, 2023
… registrations

ghstack-source-id: 676bf963dcbc0a4b7907d0c76da0bd7c7df8fc20
Pull Request resolved: #98403
…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request May 25, 2023
… registrations

ghstack-source-id: 7958af1d4d433c13833604e40268d8e5d1e813af
Pull Request resolved: #98403
@@ -5728,6 +5728,22 @@ def no_pool():
self.assertEqual(len(get_cudagraph_segments(pool)), 0)


def test_no_triton_on_import(self):
script = """import sys; import torch; torch.rand(2, device='cuda'); exit(2 if "triton' in sys.modules else 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is obviously syntactically incorrect yet the tests are green...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but I'm confused how the tests are green if one of them supposed to fail?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It fails only with the script returning error code 2, not 1. That was fixed to fail with anything nonzero.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, but let's simplify it every further: any exception during that subprocess.check_call should be treated as test failure.

…iton kernel registrations"

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request May 30, 2023
… registrations

ghstack-source-id: 30b5cec5f569abdb7401e89d5b7a4cdac709d7da
Pull Request resolved: #98403
@malfet
Copy link
Contributor

malfet commented May 31, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: PR 98403 is out of sync with the corresponding revision 6fb61e0 on branch origin/gh/nikitaved/34/orig that would be merged into main. This usually happens because there is a non ghstack change in the PR. Please sync them and try again (ex. make the changes on origin/gh/nikitaved/34/orig and run ghstack).

Details for Dev Infra team Raised by workflow job

…iton kernel registrations"




<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at 08f7a6a</samp>

This pull request adds support for triton kernels in `torch` and `torch/cuda`, and refactors and tests the existing triton kernel for BSR matrix multiplication. It also adds a test case to ensure that importing `torch` does not implicitly import `triton`.

cc alexsamardzic pearu cpuhrsch amjames bhosmer albanD mruberry jbschlosser walterddr mikaylagawarecki

[ghstack-poisoned]
nikitaved added a commit that referenced this pull request May 31, 2023
… registrations

ghstack-source-id: 488b681160a36a21fe1aa0d2844e82991e5b2c15
Pull Request resolved: #98403
@nikitaved nikitaved requested a review from albanD May 31, 2023 09:28
@malfet
Copy link
Contributor

malfet commented May 31, 2023

@pytorchbot merge

@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

shaoyf42 pushed a commit to shaoyf42/pytorch that referenced this pull request Jun 1, 2023
… registrations (pytorch#98403)

<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at 08f7a6a</samp>

This pull request adds support for triton kernels in `torch` and `torch/cuda`, and refactors and tests the existing triton kernel for BSR matrix multiplication. It also adds a test case to ensure that importing `torch` does not implicitly import `triton`.

Pull Request resolved: pytorch#98403
Approved by: https://github.com/malfet, https://github.com/cpuhrsch
alimoezzi pushed a commit to alimoezzi/pytorch that referenced this pull request Jun 3, 2023
… registrations (pytorch#98403)

<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at 08f7a6a</samp>

This pull request adds support for triton kernels in `torch` and `torch/cuda`, and refactors and tests the existing triton kernel for BSR matrix multiplication. It also adds a test case to ensure that importing `torch` does not implicitly import `triton`.

Pull Request resolved: pytorch#98403
Approved by: https://github.com/malfet, https://github.com/cpuhrsch
@facebook-github-bot facebook-github-bot deleted the gh/nikitaved/34/head branch June 8, 2023 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged module: nn Related to torch.nn module: sparse Related to torch.sparse open source release notes: sparse release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants