[JIT] min(li) max(li)#26351
Conversation
driazati
left a comment
There was a problem hiding this comment.
It would be nice to have it for all our iterables like tuple and dict too
| return max(li) | ||
|
|
||
|
|
||
| int_lists = [1], [2, 1, 2], [-3, 4, 2], [-2, -7, 1, 4], [2, 1, 0, 4] |
| } | ||
| return repeated; | ||
| } | ||
| case TypeKind::BoolType: |
There was a problem hiding this comment.
Does this fix #26036? Add it to the PR description if so
There was a problem hiding this comment.
it is in the PR description :')
There was a problem hiding this comment.
looool, I had the exact same question! I'll wait till this in, this way I don't have to fix my PR.
| } | ||
|
|
||
| template <typename T> | ||
| int listMin(Stack& stack) { |
There was a problem hiding this comment.
The copy paste isn't great, it could be 1 method that returns a std::tuple(min, max) then when used you can statically .get<1> or .get<0>
There was a problem hiding this comment.
Also i don't think doing twice as many comparisons is worth it either
There was a problem hiding this comment.
We might not necessarily save a whole lot here, since we would probably need lambda wrappers and get<0>, get<1> aren't very descriptive even in the context of Operator("min. We are probably very unlikely to ever hit an issue with these two functions to warrant the deduplication of the two?
It will already work for tuple because we allow homogenous tuples to match to lists. min() for heterogenous tuples isn't well-defined |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| "aten::reverse(" decl_type "[](a!) self) -> ()", \ | ||
| listReverse<value_type>, \ | ||
| aliasAnalysisFromSchema()), \ | ||
| Operator( \ |
There was a problem hiding this comment.
can we make these in the aten namespace instead of prim since these two have schema.
There was a problem hiding this comment.
hmmm seems like the previous one already in prim.. nvm.
Summary: Add min and max of a list to JIT. Fixes pytorch#26036 Pull Request resolved: pytorch#26351 Differential Revision: D17427547 Pulled By: eellison fbshipit-source-id: 45796b4076eef0b496b01c2cc710ec4dc15a1ee6
Add min and max of a list to JIT. Fixes #26036