high pri Jit builtins #21451
high pri Jit builtins #21451
Conversation
pytorchbot
added
oncall: jit
| _(aten, wait) \ | ||
| _(aten, save) \ | ||
| _(aten, ord) \ | ||
| _(aten, chr) \ |
There was a problem hiding this comment.
I am not sure these values are actually used. I only see them as strings not a symbols.
There was a problem hiding this comment.
Yea somehow it didn't compile without adding them here. I will double check before merging. Thanks!
There was a problem hiding this comment.
These are used as symbols in compiler.cpp, so it makes sense to add them here I think.
torch/csrc/jit/script/compiler.cpp
Outdated
| {"list", std::make_shared<BuiltinFunction>(aten::list, at::nullopt)}, | ||
| {"ord", std::make_shared<BuiltinFunction>(aten::ord, at::nullopt)}, | ||
| {"chr", std::make_shared<BuiltinFunction>(aten::chr, at::nullopt)}, | ||
| {"hex", std::make_shared<BuiltinFunction>(aten::hex, at::nullopt)}, |
There was a problem hiding this comment.
round hex and oct are magic methods, and round has a second argument for number of decimal places that defaults to 0
There was a problem hiding this comment.
@eellison I noticed the second optional argument for round but it has a weird behavior that is hard to match from cpp side. See notes in https://docs.python.org/3/library/functions.html#round.
Also I think round is rarely with that arguments, would prefer to delay implementing it unless it's necessary. :P
There was a problem hiding this comment.
round with optional args's impl in cpython is a bit long. I will make that a low pri item for following patches. https://github.com/python/cpython/blob/36dcaab7fde5d2e54cdeff5b705b5adcb27726dd/Objects/floatobject.c#L935-L1036
There was a problem hiding this comment.
round without arguments is the same thing as int, so there's not much utility in having it without the argument.
There was a problem hiding this comment.
@eellison They're different :P
round(2.6)=3
int(2.6)=2
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@pytorchbot rebase this please |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@ailzhang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: bin/hex/oct/round/chr Pull Request resolved: pytorch/pytorch#21451 Differential Revision: D15702863 Pulled By: ailzhang fbshipit-source-id: 9f69896b79e7584f12353e9f2ee2969dbe1ec6d6
6 participants
bin/hex/oct/round/chr