Adds arccos, arcsin, arctan aliases #43319
Conversation
mruberry
added
the
module: numpy
facebook-github-bot
added
the
oncall: jit
💊 CI failures summary and remediationsAs of commit e338b19 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 3 times. |
| {aten::arccosh, aten::acosh}, | ||
| {aten::arccosh_, aten::acosh_}}; | ||
| {aten::arccosh_, aten::acosh_}, | ||
| {aten::arccos, aten::acos_}, |
There was a problem hiding this comment.
shouldn't this be the non-inplace variant?
There was a problem hiding this comment.
Also: let's elaborate the test to catch errors like this just in case they occur again.
gchanan
left a comment
gchanan
left a comment
There was a problem hiding this comment.
looks good once JIT issue is addressed.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@mruberry has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@mruberry I wonder why // arcsin, alias of asin
Tensor& arcsin_out(Tensor& result, const Tensor& self) { return unary_op_impl_out(result, self, asin_stub); }
Tensor arcsin(const Tensor& self) { return unary_op_impl(self, at::asin_out); }
Tensor& arcsin_(Tensor& self) { return unary_op_impl_(self, at::asin_out); }using // Implements the "clip" alias for clamp
Tensor& clip_out(Tensor& result, const Tensor& self, optional<Scalar> min, optional<Scalar> max) {
return at::clamp_out(result, self, min, max);
}
Tensor clip(const Tensor& self, optional<Scalar> min, optional<Scalar> max) {
return at::clamp(self, min, max);
}
Tensor& clip_(Tensor& self, optional<Scalar> min, optional<Scalar> max) {
return at::clamp_(self, min, max);
}thanks ! Context: |
Hey @vfdev-5, sorry to hear you're hitting an issue. I agree having them just immediately call the asin functions would be simpler. I'll put up a PR for that now. |
|
@mruberry thanks ! Otherwise I can also send a PR with the fix if you want... |
|
Should be OK now, @vfdev-5 -- let me know. |
|
@mruberry thanks for the PR, it fixes my issue ! |
Awesome! |
These aliases are consistent with NumPy (see, for example, https://numpy.org/doc/stable/reference/generated/numpy.arccos.html?highlight=acos).
Note that PyTorch's existing names are consistent with Python (see https://docs.python.org/3.10/library/math.html?highlight=acos#math.acos) and C++ (see, for example, https://en.cppreference.com/w/cpp/numeric/math/acos).