-
Notifications
You must be signed in to change notification settings - Fork 85
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
update torch.where
to throw error for single input
#192
Conversation
torch.where
to handle single input
torch.where
to handle single inputtorch.where
to throw error for single input
…Error due to torch.where
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good except for one small nit about the type annotations on where
-- I made a comment for you review
Hey @mruberry! Thanks for the review :) Just updated my code based on your comments! Can't wait to see my first PR getting merged 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Thanks @k223kim!
Just one more question @mruberry! It says "required statuses must pass before merging". Is there anything else I should be working on to merge this PR? It's my first PR so I am not really familiar with this process. ;) |
Nope, nothing else you need to do, @k223kim. Sorry for the confusion. @t-vi @Borda we should probably think about clarifying the merge process somewhere |
Thank you @k223kim @mruberry . @mruberry Yes, maybe you can add a little paragraph to @nikitaved (great) contributing section https://lightning-thunder.readthedocs.io/en/latest/advanced/contrib_thunder.html Thank you. |
Not sure I'd know what to write. |
@mruberry When it is ready for my PR to be merged?
If so, your PR is ready to be merged! 🚀 |
That sounds like a great start, @k223kim! |
Before submitting
What does this PR do?
Fixes #146. It throws a more descriptive error saying that Thunder is yet to support single input (condition only)
torch.where
.PR review
Anyone in the community is free to review the PR once the tests have passed.
Did you have fun?
Loving every bit of this! 🚀