-
Notifications
You must be signed in to change notification settings - Fork 114
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
Add torch implementation of IfElse #974
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #974 +/- ##
=======================================
Coverage 81.74% 81.74%
=======================================
Files 183 183
Lines 47733 47742 +9
Branches 11616 11617 +1
=======================================
+ Hits 39020 39029 +9
- Misses 6518 6520 +2
+ Partials 2195 2193 -2
|
|
||
def ifelse(cond, *true_and_false, n_outs=n_outs): | ||
if cond: | ||
return torch.stack(true_and_false[:n_outs]) |
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.
We don't need to stack, and shouldn't, because outputs can have different dimensions / sizes
a = scalar("a") | ||
x = ifelse(a < 0.4, tuple(np.r_[p1_vals, p2_vals]), tuple(np.r_[p2_vals, p1_vals])) | ||
x_fg = FunctionGraph([a], x) |
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.
If you want to test twice, to cover the false case, do a case where the multiple outputs are not something that can be stacked (say (pt.zeros((3, 5), pt.ones(2,))
.
I would have the other test case return a single output, to test the single output case as well
Thanks @Ch0ronomato |
Co-authored-by: Ian Schweer <[email protected]>
Description
Add the IfElse op support in torch (reopened cause i
git screwup
my old branch)Related Issue
Checklist
Type of change