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
threshold
: port to structured
#57810
Conversation
💊 CI failures summary and remediationsAs of commit 016b065 (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 to the (internal) Dr. CI Users group. |
Codecov Report
@@ Coverage Diff @@
## master #57810 +/- ##
==========================================
- Coverage 76.84% 76.83% -0.01%
==========================================
Files 1986 1986
Lines 197889 197890 +1
==========================================
- Hits 152068 152058 -10
- Misses 45821 45832 +11 |
aten/src/ATen/native/Activation.cpp
Outdated
namespace at { | ||
namespace meta { | ||
TORCH_META_FUNC(threshold)(const Tensor& self, const Scalar& threshold, const Scalar& value) { | ||
set_output(0, self.sizes(), {}, self.options().memory_format(self.suggest_memory_format()), self.names()); |
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.
This will twing named tensor warnings so you should do self.has_names() ? self.names() : at::ArrayRef<Dimname>{}
instead. Sorry it's dumb and we should fix this.
cc @zou3519
You should do this one a little differently. This is a TensorIterator op, so you should use the TensorIteratorBase recipe from https://github.com/pytorch/rfcs/blob/rfc-0005/RFC-0005-structured-kernel-definitions.md#handling-tensoriterator and build the tensor iterator inside the meta function itself. This will prevent you from having to manually define the |
That will also let you deduplicate the TensorIterator construction calls in threshold_out_cuda and threshold_out |
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.
more refactorrr
62b21b4
to
016b065
Compare
@ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: Related pytorch#55070 Port threshold and threshold_backward to structure Pull Request resolved: pytorch#57810 Reviewed By: agolynski Differential Revision: D28382716 Pulled By: ezyang fbshipit-source-id: 8d0702ad074b52e8512524d9807c93bfe04c51d6
Related #55070
Port threshold and threshold_backward to structure