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.

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

Adding a decomposition and a test for triu #1631

Merged
merged 6 commits into from
Jan 10, 2025
Merged

Adding a decomposition and a test for triu #1631

merged 6 commits into from
Jan 10, 2025

Conversation

protonu
Copy link
Contributor

@protonu protonu commented Jan 10, 2025

This PR decomposes triu in the same manner as tril. This also extends test_ops.py to cover triu

What does this PR do?

Fixes # (issue).
#1627

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

@protonu protonu mentioned this pull request Jan 10, 2025
Closed
beverlylytle
beverlylytle reviewed Jan 10, 2025
View reviewed changes
Copy link
Collaborator

@beverlylytle beverlylytle left a comment

Choose a reason for hiding this comment

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

Thanks!

This looks good so far, but the operator also needs to be registered in the torch executor. See here and here for how it is done for tril.

thunder/tests/opinfos.py Outdated Show resolved Hide resolved
thunder/torch/__init__.py Outdated Show resolved Hide resolved
@protonu protonu requested a review from beverlylytle January 10, 2025 13:06
@protonu
Copy link
Contributor Author

protonu commented Jan 10, 2025

Thanks!

This looks good so far, but the operator also needs to be registered in the torch executor. See here and here for how it is done for tril.

Thanks! Please let me know if I need to register this anywhere else.

@protonu protonu requested a review from beverlylytle January 10, 2025 13:15
@protonu protonu requested a review from nikitaved January 10, 2025 14:05
@protonu protonu requested a review from nikitaved January 10, 2025 17:13
mruberry
mruberry reviewed Jan 10, 2025
View reviewed changes
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Let's give the tests a chance to run

thunder/torch/__init__.py Show resolved Hide resolved
@mruberry mruberry enabled auto-merge (squash) January 10, 2025 18:03
mruberry
mruberry approved these changes Jan 10, 2025
View reviewed changes
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Tests look good; thanks for adding triu, @protonu!

@mruberry mruberry merged commit 5864232 into Lightning-AI:main Jan 10, 2025
41 checks passed
mruberry
mruberry approved these changes Jan 10, 2025
View reviewed changes
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Tests look good; thanks for adding triu, @protonu!

@kevinstephano kevinstephano mentioned this pull request Jan 10, 2025
Open
riccardofelluga pushed a commit that referenced this pull request Jan 27, 2025
@protonu @riccardofelluga
Adding a decomposition and a test for triu (#1631)
3e24ca1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mruberry mruberry mruberry approved these changes

@lantiga lantiga Awaiting requested review from lantiga lantiga is a code owner

@t-vi t-vi Awaiting requested review from t-vi t-vi is a code owner

@beverlylytle beverlylytle Awaiting requested review from beverlylytle

@nikitaved nikitaved Awaiting requested review from nikitaved

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@protonu @nikitaved @mruberry @beverlylytle