Fix inconsistent python/cpp API behavior for if_then_else, power #3829
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
|
@kevinthesun @icemelon9 |
kevinthesun
requested changes
Aug 24, 2019
| @@ -419,7 +419,7 @@ def power(x, y): | |||
| z : Expr | |||
| The result. | |||
| """ | |||
| return call_pure_intrin(x.dtype, "pow", x, y) | |||
| return _make._OpPow(convert(x), convert(y)) | |||
There was a problem hiding this comment.
Add a test case to check the type conversion.
| if cond.dtype != "bool": | ||
| raise TypeError("The condition's data type has to be bool") | ||
| return call_pure_intrin(t.dtype, "tvm_if_then_else", cond, t, f) | ||
| return _make._OpIfThenElse(convert(cond), convert(t), convert(f)) |
|
@kevinthesun I've added the checks. |
|
Also, I find that the behavior of For example: import numpy as np
import tvm
print((tvm.var(dtype='int32') + tvm.var(dtype='float16')).dtype)
# result: float16
print((tvm.var(dtype='int32') + tvm.var(dtype='float32')).dtype)
# result: float32
print((np.array(1).astype('int32') + np.array(1).astype('float32')).dtype)
# result: float64
print((np.array(1).astype('int32') + np.array(1).astype('float16')).dtype)
# result: float64Seems that numpy promotes the float to the highest precision whenever it meets the integer + float. |
kevinthesun
approved these changes
Aug 26, 2019
|
Thanks! |
wweic
pushed a commit
to wweic/tvm
that referenced
this pull request
Sep 16, 2019
…che#3829) * fix inconsistent python/cpp APIs for if_then_else * fix error message * fix power consistency * fix * fix bug * add test
wweic
pushed a commit
to wweic/tvm
that referenced
this pull request
Sep 16, 2019
…che#3829) * fix inconsistent python/cpp APIs for if_then_else * fix error message * fix power consistency * fix * fix bug * add test
wweic
pushed a commit
to neo-ai/tvm
that referenced
this pull request
Sep 16, 2019
…che#3829) * fix inconsistent python/cpp APIs for if_then_else * fix error message * fix power consistency * fix * fix bug * add test
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.
In the C++ side, the if_then_else will call BinaryOpMatchTypes, which will try to convert the dtypes of the inner expressions to be the same:
https://github.com/dmlc/tvm/blob/d05893b3aaddb098780a2fbaf2d2af81bb50d393/src/lang/expr_operator.cc#L242
However, in the python side, there is no such conversion, which makes the behavior inconsistent.
The same issue occurs for power. Thus, fix by directly registering the APIs inside _make.
To give an example of the inconsistent behavior, consider the following example:
In the original version, the result will be
If we use the C++ API, it will be the following, in which b will be automatically casted to float32.