support vs2019 compilation in windows #38719
Conversation
| Thanks for your contribution! |
betterpig force-pushed the vs2019_part1 branch from 89e7009 to e664f23 Compare betterpig force-pushed the vs2019_part1 branch from e664f23 to 7a921d6 Compare betterpig changed the title betterpig force-pushed the vs2019_part1 branch from 6a3a954 to 3a4ca51 Compare 
zhwesky2010 left a comment
zhwesky2010 left a comment There was a problem hiding this comment.
综合评估下pow fmin fmax这几个OP有没有不兼容风险，结果是否和之前一致
| elseif(WIN32) | ||
| SET(PROTOBUF_REPOSITORY ${GIT_URL}/protocolbuffers/protobuf.git) | ||
| # Change the tag to support building with vs2019 | ||
| SET(PROTOBUF_TAG 01a05a53f40ca2ac5f0af10c6cc0810bee39b792) |
| }; | ||
| | ||
| template <> | ||
| struct FMaxFunctor<int> { |
There was a problem hiding this comment.
std::fmax 不能直接处理int类型的输入吗，还是说原本就有一个隐式的类型转换，在VS2019时必须变为显式？
There was a problem hiding this comment.
应该是在VS2019， int / int64_t不能隐式转换到float / double，导致CUDA找不到对应的函数声明，标准库的fmax没有这个问题。在CUDA10.2的机器上，使用VS2019编译，也存在这个问题。
此次修改针对int / int64_t类型，先将其精度提高，在调用fmax获得结果后，再通过lrint函数转换回原类型。
因为是整型，操作是不涉及到计算的比大小，再加上lrint能圆整回最接近的正数，因此不会影响结果。
| inline HOSTDEVICE T operator()(const T args[]) const { | ||
| return std::llrint(std::pow(args[0], args[1])); | ||
| return std::llrint( | ||
| std::pow(static_cast<double>(args[0]), static_cast<double>(args[1]))); |
There was a problem hiding this comment.
CUDA pow这里的计算是否会导致结果不同，这样的话会有不兼容风险
There was a problem hiding this comment.
这是只是将 int / int64_t 转换为 double类型后，进行pow运算，然后再用 llrint 函数取最接近的整数。因此不会导致结果不同，也就不会有非兼容性风险。
| if (std::is_integral<T>::value) { | ||
| return std::llrint(std::pow(a, b)); | ||
| return std::llrint( | ||
| std::pow(static_cast<double>(a), static_cast<double>(b))); |
There was a problem hiding this comment.
这是只是将 int / int64_t 转换为 double类型后，进行pow运算，然后再用 llrint 函数取最接近的整数。因此不会导致结果不同，也就不会有非兼容性风险。
| }; | ||
| | ||
| template <typename T> | ||
| struct PowGradDY<T, typename std::enable_if<std::is_integral<T>::value>::type> { |
There was a problem hiding this comment.
对于PowGradDX和PowGradDY的修改，有两个方面：
- 将 int / int64_t 转换为 double类型，以调用CUDA的pow 函数
- 加上 llrint 做圆整，而不是使用隐式的类型转换 float -> int, double -> long long。因为隐式的类型转换默认是去尾法，显然是不合理的，应该用 llrint，取最接近的整数。举个例子，设x为6，y为1，则PowGradDY的实际结果为log(6)*6=10.75... 如果不用 llrint，PowGradDY的返回值为 10 ，使用 llrint 后，则为 11.
因此，第2个修改不是升级VS2019所必需的。若是考虑到兼容问题，可以去掉第2个修改。
| # dx = dout * y * pow(x, y-1) | ||
| self.grad_x = self.grad_res * self.y * (self.x | ||
| **(self.y - 1)).astype("int") | ||
| self.grad_x = (np.rint(self.grad_res * self.y * self.x |
There was a problem hiding this comment.
看起来OP结果会改变 这样会有一些不兼容风险。如果要加很多 显式类型变换，要确保和以前的隐式变换一样
There was a problem hiding this comment.
单测的修改是为了和PowGradDY的第2个修改对应，具体说明见上一条。
astype("int")也是采用去尾法，需要先取最接近的整数，再转换为int。
betterpig force-pushed the vs2019_part1 branch from 0298b29 to a0d9b95 Compare betterpig force-pushed the vs2019_part1 branch 2 times, most recently from afa946a to 902e82d Compare betterpig force-pushed the vs2019_part1 branch from 902e82d to d1c8cac Compare 
好的 |
| 咋样了，现在支持VS2019编译了没？ |
| 已支持VS2019编译 |
第三方库的支持问题，有解决吗？第三方库下载不下来，建议使用链接的方式，而不是直接git下载的方式 |
| 现在使用VS2019编译，不存在第三方库的支持问题。考虑到版权问题，第三方库尽量使用git clone方式获取源码。 |
3 participants
PR types
Bug fixes
PR changes
Others
Describe
fix vs2019 compilation problem in windows
change protobuf's tag to avoid building failure when building with vs2019 in windows.
cuda only support some combination of args list for these fuction, see this. so i use template specialization to transfer type to call cuda math api rightly when data type is int, int64_t.
It is not correct that
return dout * std::log(x) * std::pow(x, y);when x and y is integer. Since log(int) may be decimal, the thus final result is decimal. While the return value will be floored to 2 and it is wrong. So I add llrint for both cpu and gpu code, and for test_elementwise_pow_op also.