Add clamp conversion functionality #293
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
component: conversion
3 tasks
narendasan
reviewed
Jan 26, 2021
| @@ -144,6 +144,41 @@ auto element_wise_registrations TRTORCH_UNUSED = | |||
| LOG_DEBUG("Output tensor shape: " << out->getDimensions()); | |||
| return true; | |||
| }}) | |||
| .pattern({"aten::clamp(Tensor self, Scalar? min=None, Scalar? max=None) -> (Tensor)", | |||
There was a problem hiding this comment.
Could we possibly lower to hardtanh or is the functionality different?
There was a problem hiding this comment.
hardtanh looks a bit different https://pytorch.org/docs/stable/generated/torch.nn.Hardtanh.html
hardtanh compares the elements to -1 and 1 and replaces with default values (thresholds) where as clamp compares the input elements to thresholds directly. They both probably can match in certain scenarios
There was a problem hiding this comment.
hardtanh lets you configure the min and max values. really my point is we should be able to lower clamp and hardtanh to the same clip activation converter. This is something we could leave for later though
There was a problem hiding this comment.
Ideally our converter library should be as small as possible and we should do as much as we can through lowering
| // Compute min(max(min_threshold, input), max_threshold) | ||
| auto self = args[0].ITensorOrFreeze(ctx); | ||
| auto clamp_layer_out = self; | ||
| if (args[1].isIValue() && args[1].IValue()->isScalar()) { |
There was a problem hiding this comment.
I think we might be able to do !isNone on the IValue instead of two checks
There was a problem hiding this comment.
If I remember correctly, isNone() didn't work for me. I will double check again.
There was a problem hiding this comment.
The op does basically this min(max(min_threshold, input), max_threshold)
In the case when min_threshold or max_threshold is not given, args[1] or args[2] will be registered as an IValue.
The isNone() check will check if the type is one of { kITensor, kIValue, kNone }. The type in this case would be IValue so isNone() will not return true.
narendasan
approved these changes
Jan 28, 2021
|
@peri044 can you resolve merge conflicts? Ill merge after that. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
There are some changes that do not conform to C++ style guidelines:
diff --git a/workspace/tests/core/conversion/converters/test_element_wise.cpp b/tmp/changes.txt
index 444a8d3..ea88bad 100644
--- a/workspace/tests/core/conversion/converters/test_element_wise.cpp
+++ b/tmp/changes.txt
@@ -173,21 +173,21 @@ TEST(Converters, ATenClampMinMaxConvertsCorrectly) {
return (%4))IR";
pointwise_test_helper(graph, true);
-TEST(Converters, ATenNeTensorConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenNeTensorConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor,
%y.1 : Tensor):
%3 : Tensor = aten::ne(%x.1, %y.1)
return (%3))IR";
- pointwise_test_helper(graph, false, false, {3, 4}, {3, 4});
- pointwise_test_helper(graph, false, true, {3, 4}, {3, 4});
-}
+ pointwise_test_helper(graph, false, false, {3, 4}, {3, 4});
+ pointwise_test_helper(graph, false, true, {3, 4}, {3, 4});
+ }
-TEST(Converters, ATenNeScalarConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenNeScalarConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor):
%2 : int = prim::Constant[value=2]()
%3 : Tensor = aten::ne(%x.1, %2)
return (%3))IR";
- pointwise_test_helper(graph, true, false, {3, 4, 2});
-}
+ pointwise_test_helper(graph, true, false, {3, 4, 2});
+ }
ERROR: Some files do not conform to style guidelines
There was a problem hiding this comment.
There are some changes that do not conform to C++ style guidelines:
diff --git a/workspace/tests/core/conversion/converters/test_element_wise.cpp b/tmp/changes.txt
index 444a8d3..ea88bad 100644
--- a/workspace/tests/core/conversion/converters/test_element_wise.cpp
+++ b/tmp/changes.txt
@@ -173,21 +173,21 @@ TEST(Converters, ATenClampMinMaxConvertsCorrectly) {
return (%4))IR";
pointwise_test_helper(graph, true);
-TEST(Converters, ATenNeTensorConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenNeTensorConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor,
%y.1 : Tensor):
%3 : Tensor = aten::ne(%x.1, %y.1)
return (%3))IR";
- pointwise_test_helper(graph, false, false, {3, 4}, {3, 4});
- pointwise_test_helper(graph, false, true, {3, 4}, {3, 4});
-}
+ pointwise_test_helper(graph, false, false, {3, 4}, {3, 4});
+ pointwise_test_helper(graph, false, true, {3, 4}, {3, 4});
+ }
-TEST(Converters, ATenNeScalarConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenNeScalarConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor):
%2 : int = prim::Constant[value=2]()
%3 : Tensor = aten::ne(%x.1, %2)
return (%3))IR";
- pointwise_test_helper(graph, true, false, {3, 4, 2});
-}
+ pointwise_test_helper(graph, true, false, {3, 4, 2});
+ }
ERROR: Some files do not conform to style guidelines
peri044
force-pushed
the
clamp
branch
2 times, most recently
from
7b076d2 to
96dbbf3
Compare
Signed-off-by: Dheeraj Peri <[email protected]> Apply linting Signed-off-by: Dheeraj Peri <[email protected]> Fix merge conflicts Signed-off-by: Dheeraj Peri <[email protected]>
There was a problem hiding this comment.
There are some changes that do not conform to C++ style guidelines:
diff --git a/workspace/tests/core/conversion/converters/test_element_wise.cpp b/tmp/changes.txt
index 0759b59..377c189 100644
--- a/workspace/tests/core/conversion/converters/test_element_wise.cpp
+++ b/tmp/changes.txt
@@ -163,37 +163,37 @@ TEST(Converters, ATenNeScalarConvertsCorrectly) {
pointwise_test_helper(graph, true, false, {3, 4, 2});
;
-TEST(Converters, ATenClampMinConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenClampMinConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor):
%2 : int = prim::Constant[value=-2]()
%3 : None = prim::Constant()
%4 : Tensor = aten::clamp(%x.1, %2, %3)
return (%4))IR";
- pointwise_test_helper(graph, true);
-}
+ pointwise_test_helper(graph, true);
+ }
-TEST(Converters, ATenClampMaxConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenClampMaxConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor):
%2 : int = prim::Constant[value=3]()
%3 : None = prim::Constant()
%4 : Tensor = aten::clamp(%x.1, %3, %2)
return (%4))IR";
- pointwise_test_helper(graph, true);
-}
+ pointwise_test_helper(graph, true);
+ }
-TEST(Converters, ATenClampMinMaxConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenClampMinMaxConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor):
%2 : int = prim::Constant[value=3]()
%3 : int = prim::Constant[value=-2]()
%4 : Tensor = aten::clamp(%x.1, %3, %2)
return (%4))IR";
- pointwise_test_helper(graph, true);
-}
+ pointwise_test_helper(graph, true);
+ }
-TEST(Converters, ATenNeTensorConvertsCorrectly) {
+ TEST(Converters, ATenNeTensorConvertsCorrectly) {
const auto graph = R"IR(
graph(%x.1 : Tensor,
%y.1 : Tensor):
@@ -201,13 +201,13 @@ TEST(Converters, ATenNeTensorConvertsCorrectly) {
return (%3))IR";
pointwise_test_helper(graph, false, false, {3, 4}, {3, 4});
pointwise_test_helper(graph, false, true, {3, 4}, {3, 4});
-}
+ }
-TEST(Converters, ATenNeScalarConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenNeScalarConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor):
%2 : int = prim::Constant[value=2]()
%3 : Tensor = aten::ne(%x.1, %2)
return (%3))IR";
- pointwise_test_helper(graph, true, false, {3, 4, 2});
-}
+ pointwise_test_helper(graph, true, false, {3, 4, 2});
+ }
ERROR: Some files do not conform to style guidelines
There was a problem hiding this comment.
There are some changes that do not conform to C++ style guidelines:
diff --git a/workspace/tests/core/conversion/converters/test_element_wise.cpp b/tmp/changes.txt
index 0c28da7..edfb185 100644
--- a/workspace/tests/core/conversion/converters/test_element_wise.cpp
+++ b/tmp/changes.txt
@@ -163,32 +163,32 @@ TEST(Converters, ATenNeScalarConvertsCorrectly) {
pointwise_test_helper(graph, true, false, {3, 4, 2});
;
-TEST(Converters, ATenClampMinConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenClampMinConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor):
%2 : int = prim::Constant[value=-2]()
%3 : None = prim::Constant()
%4 : Tensor = aten::clamp(%x.1, %2, %3)
return (%4))IR";
- pointwise_test_helper(graph, true);
-}
+ pointwise_test_helper(graph, true);
+ }
-TEST(Converters, ATenClampMaxConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenClampMaxConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor):
%2 : int = prim::Constant[value=3]()
%3 : None = prim::Constant()
%4 : Tensor = aten::clamp(%x.1, %3, %2)
return (%4))IR";
- pointwise_test_helper(graph, true);
-}
+ pointwise_test_helper(graph, true);
+ }
-TEST(Converters, ATenClampMinMaxConvertsCorrectly) {
- const auto graph = R"IR(
+ TEST(Converters, ATenClampMinMaxConvertsCorrectly) {
+ const auto graph = R"IR(
graph(%x.1 : Tensor):
%2 : int = prim::Constant[value=3]()
%3 : int = prim::Constant[value=-2]()
%4 : Tensor = aten::clamp(%x.1, %3, %2)
return (%4))IR";
- pointwise_test_helper(graph, true);
-}
+ pointwise_test_helper(graph, true);
+ }
ERROR: Some files do not conform to style guidelines
|
Resolved conflicts |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add clamp conversion
Fixes #274
Type of change
Checklist: