revise tan op #6567
Merged
revise tan op #6567
Conversation
This file contains 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
pelszkow
requested review from
lazarevevgeny and
ilyachur
and removed request for
pelszkow
rkazants
reviewed
Jul 9, 2021
rkazants
reviewed
Jul 12, 2021
| @@ -6,32 +6,29 @@ | |||
|
|
|||
| **Short description**: *Tan* performs element-wise tangent operation with given tensor. | |||
|
|
|||
| **Attributes**: | |||
| **Detailed description**: Operation takes one input tensor and performs the element-wise tangent function on a given input tensor, based on the following mathematical formula: | |||
There was a problem hiding this comment.
please clarify input is in radians or degrees.
What value will be returned in case inf floating values?
There was a problem hiding this comment.
done, update the document
There was a problem hiding this comment.
@rkazants could you help to review, thanks
tiger100256-hu
force-pushed
the
hu/revise_op_tan
branch
from
fff9d33 to
e8f3365
Compare
ilyachur
reviewed
Jul 19, 2021
ngraph/test/backend/tan.in.cpp
Outdated
| @@ -62,3 +62,24 @@ NGRAPH_TEST(${BACKEND_NAME}, tan) | |||
| -1.15782128f}, | |||
| read_vector<float>(result))); | |||
| } | |||
|
|
|||
|
|
|||
| NGRAPH_TEST(${BACKEND_NAME}, tan_int32) | |||
There was a problem hiding this comment.
Could you use template reference tests instead of backend tests?
tiger100256-hu
force-pushed
the
hu/revise_op_tan
branch
from
e8f3365 to
65cd9c3
Compare
ilyachur
reviewed
Jul 21, 2021
ngraph/test/backend/tan.in.cpp
Outdated
| @@ -34,7 +34,7 @@ using namespace ngraph; | |||
|
|
|||
| static string s_manifest = "${MANIFEST}"; | |||
|
|
|||
| NGRAPH_TEST(${BACKEND_NAME}, tan) | |||
| NGRAPH_TEST(${BACKEND_NAME}, tan_float) | |||
There was a problem hiding this comment.
Could you remove tan backend test? And use only template reference tests instead?
| } | ||
|
|
||
| std::vector<TanParams> generateTanCombinedParams() { | ||
| const std::vector<std::vector<TanParams>> tanTypeParams {generateTanParamsInt<element::Type_t::i32>(ngraph::element::i32), |
There was a problem hiding this comment.
Could you cover all supported types?
tiger100256-hu
commented
Jul 22, 2021
| std::vector<int32_t> {2, -2, 0, 2, -2}), | ||
| TanParams(ngraph::PartialShape {5}, ngraph::element::i64, std::vector<int64_t> {-2, -1, 0, 1, 2}, | ||
| std::vector<int64_t> {2, -2, 0, 2, -2}), | ||
| TanParams(ngraph::PartialShape {5}, ngraph::element::u32, std::vector<uint32_t> {1, 2, 3, 4, 5}, |
There was a problem hiding this comment.
@ilyachur I don't know if need to support u32 u64, it's very strange to transform int to uint here
There was a problem hiding this comment.
@ilyachur for example -2 to 0xFFFFFFFFFFFFFFFF - 1
There was a problem hiding this comment.
Why do you need negative numbers for uint data types?
Can you just change reference values?
There was a problem hiding this comment.
tan(2)=-2, but the output type is uint
docs/ops/arithmetic/Tan_1.md
Outdated
|
|
||
| **Types** | ||
|
|
||
| * *T*: any numeric type. | ||
| * *T*: int32,int64,uint32,uint64,float16,float32 |
There was a problem hiding this comment.
We do not specify types like this. As plugins may support different types. Please change this to -> *T*: any supported numeric type.
There was a problem hiding this comment.
Update the document. Are the test case about uint ok ?
ilyachur
approved these changes
Aug 3, 2021
|
Please fix CI |
tiger100256-hu
force-pushed
the
hu/revise_op_tan
branch
from
dcfd68c to
5c1835f
Compare
Signed-off-by: Hu, Yuan2 <[email protected]>
add examples in desciption add the unit of measure clear input type Signed-off-by: Hu, Yuan2 <[email protected]>
Signed-off-by: Hu, Yuan2 <[email protected]>
remove the float test in backend Signed-off-by: Hu, Yuan2 <[email protected]>
change type to any supported numeric type Signed-off-by: Hu, Yuan2 <[email protected]>
Signed-off-by: Hu, Yuan2 <[email protected]>
|
@ilyachur ci is fixed |
v-Golubev
pushed a commit
to v-Golubev/openvino
that referenced
this pull request
Aug 12, 2021
* revise tan op Signed-off-by: Hu, Yuan2 <[email protected]> * update doc add examples in desciption add the unit of measure clear input type Signed-off-by: Hu, Yuan2 <[email protected]> * add template plugin test case for int type Signed-off-by: Hu, Yuan2 <[email protected]> * add template plugin test case for uint and float remove the float test in backend Signed-off-by: Hu, Yuan2 <[email protected]> * modify document change type to any supported numeric type Signed-off-by: Hu, Yuan2 <[email protected]> * fix compile error in openvino-lin Signed-off-by: Hu, Yuan2 <[email protected]>
rnugmanx
pushed a commit
to rnugmanx/openvino
that referenced
this pull request
Aug 26, 2021
* revise tan op Signed-off-by: Hu, Yuan2 <[email protected]> * update doc add examples in desciption add the unit of measure clear input type Signed-off-by: Hu, Yuan2 <[email protected]> * add template plugin test case for int type Signed-off-by: Hu, Yuan2 <[email protected]> * add template plugin test case for uint and float remove the float test in backend Signed-off-by: Hu, Yuan2 <[email protected]> * modify document change type to any supported numeric type Signed-off-by: Hu, Yuan2 <[email protected]> * fix compile error in openvino-lin Signed-off-by: Hu, Yuan2 <[email protected]>
andrei-cv
pushed a commit
to andrei-cv/openvino
that referenced
this pull request
Aug 30, 2021
* revise tan op Signed-off-by: Hu, Yuan2 <[email protected]> * update doc add examples in desciption add the unit of measure clear input type Signed-off-by: Hu, Yuan2 <[email protected]> * add template plugin test case for int type Signed-off-by: Hu, Yuan2 <[email protected]> * add template plugin test case for uint and float remove the float test in backend Signed-off-by: Hu, Yuan2 <[email protected]> * modify document change type to any supported numeric type Signed-off-by: Hu, Yuan2 <[email protected]> * fix compile error in openvino-lin Signed-off-by: Hu, Yuan2 <[email protected]>
akuporos
pushed a commit
to akuporos/openvino
that referenced
this pull request
Sep 29, 2021
* revise tan op Signed-off-by: Hu, Yuan2 <[email protected]> * update doc add examples in desciption add the unit of measure clear input type Signed-off-by: Hu, Yuan2 <[email protected]> * add template plugin test case for int type Signed-off-by: Hu, Yuan2 <[email protected]> * add template plugin test case for uint and float remove the float test in backend Signed-off-by: Hu, Yuan2 <[email protected]> * modify document change type to any supported numeric type Signed-off-by: Hu, Yuan2 <[email protected]> * fix compile error in openvino-lin Signed-off-by: Hu, Yuan2 <[email protected]>
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.
Signed-off-by: Hu, Yuan2 [email protected]
Details:
Tickets: