gemini-code-assist[bot] commented on code in PR #282:
URL: https://github.com/apache/tvm-ffi/pull/282#discussion_r2549831087
##########
python/tvm_ffi/utils/_build_optional_torch_c_dlpack.py:
##########
@@ -389,7 +391,7 @@
TORCH_CHECK(false, "Unsupported kDLFloat8_e4m3fnuz bits ",
std::to_string(dtype.bits));
}
break;
-#if TORCH_VERSION_MAJOR >= 2 && TORCH_VERSION_MINOR >= 8
+#if TORCH_VERSION_MAJOR >= 2 && TORCH_VERSION_MINOR >= 7
Review Comment:
This version check is also not robust against future major version bumps of
PyTorch. For example, it will fail for version 3.0.0. It should be updated to a
more robust form.
```suggestion
#if (TORCH_VERSION_MAJOR > 2) || (TORCH_VERSION_MAJOR == 2 &&
TORCH_VERSION_MINOR >= 7)
```
##########
python/tvm_ffi/utils/_build_optional_torch_c_dlpack.py:
##########
@@ -125,10 +125,12 @@
case ScalarType::Float8_e4m3fnuz:
dtype.code = DLDataTypeCode::kDLFloat8_e4m3fnuz;
break;
-#if TORCH_VERSION_MAJOR >= 2 && TORCH_VERSION_MINOR >= 8
+#if TORCH_VERSION_MAJOR >= 2 && TORCH_VERSION_MINOR >= 7
case ScalarType::Float8_e8m0fnu:
dtype.code = DLDataTypeCode::kDLFloat8_e8m0fnu;
break;
+#endif
+#if TORCH_VERSION_MAJOR >= 2 && TORCH_VERSION_MINOR >= 8
Review Comment:
Similar to my other comment, this version check is not robust for future
major versions of PyTorch. For example, it will fail for version 3.0.0. Please
use a more robust check.
```suggestion
#if (TORCH_VERSION_MAJOR > 2) || (TORCH_VERSION_MAJOR == 2 &&
TORCH_VERSION_MINOR >= 8)
```
##########
python/tvm_ffi/utils/_build_optional_torch_c_dlpack.py:
##########
@@ -399,6 +401,8 @@
TORCH_CHECK(false, "Unsupported kDLFloat8_e8m0fnu bits ",
std::to_string(dtype.bits));
}
break;
+#endif
+#if TORCH_VERSION_MAJOR >= 2 && TORCH_VERSION_MINOR >= 8
Review Comment:
This new version check has the same robustness issue for future major
versions of PyTorch. Please use a more robust check to prevent issues with
versions like 3.0.0.
```suggestion
#if (TORCH_VERSION_MAJOR > 2) || (TORCH_VERSION_MAJOR == 2 &&
TORCH_VERSION_MINOR >= 8)
```
##########
python/tvm_ffi/utils/_build_optional_torch_c_dlpack.py:
##########
@@ -125,10 +125,12 @@
case ScalarType::Float8_e4m3fnuz:
dtype.code = DLDataTypeCode::kDLFloat8_e4m3fnuz;
break;
-#if TORCH_VERSION_MAJOR >= 2 && TORCH_VERSION_MINOR >= 8
+#if TORCH_VERSION_MAJOR >= 2 && TORCH_VERSION_MINOR >= 7
Review Comment:
The version check `TORCH_VERSION_MAJOR >= 2 && TORCH_VERSION_MINOR >= 7` is
not robust for future major versions of PyTorch. For example, it will fail for
version 3.0.0 (evaluating to `3 >= 2 && 0 >= 7`, which is false). A more robust
check should be used. This issue is present in other version checks in this
file as well.
```suggestion
#if (TORCH_VERSION_MAJOR > 2) || (TORCH_VERSION_MAJOR == 2 &&
TORCH_VERSION_MINOR >= 7)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
