-
Notifications
You must be signed in to change notification settings - Fork 206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not rely on conversions between float and extended floating point types #2046
Do not rely on conversions between float and extended floating point types #2046
Conversation
…types The issue we have is that our tests rely extensively on those conversions which makes it incredibly painfull to test
82f1c05
to
a994cc5
Compare
🟨 CI finished in 8h 59m: Pass: 99%/417 | Total: 6d 08h | Avg: 22m 00s | Max: 1h 08m | Hits: 71%/522268
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
🏃 Runner counts (total jobs: 417)
# | Runner |
---|---|
305 | linux-amd64-cpu16 |
61 | linux-amd64-gpu-v100-latest-1 |
28 | linux-arm64-cpu16 |
23 | windows-amd64-cpu16 |
🟩 CI finished in 1d 06h: Pass: 100%/417 | Total: 6d 10h | Avg: 22m 10s | Max: 1h 08m | Hits: 71%/524869
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
🏃 Runner counts (total jobs: 417)
# | Runner |
---|---|
305 | linux-amd64-cpu16 |
61 | linux-amd64-gpu-v100-latest-1 |
28 | linux-arm64-cpu16 |
23 | windows-amd64-cpu16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, except for:
@@ -37,47 +37,47 @@ _LIBCUDACXX_BEGIN_NAMESPACE_STD | |||
// trigonometric functions | |||
inline _LIBCUDACXX_INLINE_VISIBILITY __nv_bfloat16 sin(__nv_bfloat16 __v) | |||
{ | |||
NV_IF_ELSE_TARGET(NV_IS_DEVICE, (return ::hsin(__v);), (return __nv_bfloat16(::sin(float(__v)));)) | |||
NV_IF_ELSE_TARGET(NV_IS_DEVICE, (return ::hsin(__v);), (return __float2bfloat16(::sin(__bfloat162float(__v)));)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important: The host path does not seem right. The code calls the C library's ::sin
here, which promotes the argumet to a double
. We should either call ::sinf
or std::sin
here.
Applies to several math functions below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created issue: #2078
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @bernhardmgruber is right about the host paths I think, but this was preexisting condition of how I wrote that code in the first place, so while we should fix that, it shouldn't block this.
…types (NVIDIA#2046) The issue we have is that our tests rely extensively on those conversions which makes it incredibly painfull to test
…types (NVIDIA#2046) The issue we have is that our tests rely extensively on those conversions which makes it incredibly painfull to test
…types (#2046) The issue we have is that our tests rely extensively on those conversions which makes it incredibly painfull to test
…types (#2046) The issue we have is that our tests rely extensively on those conversions which makes it incredibly painfull to test
…types (#2046) The issue we have is that our tests rely extensively on those conversions which makes it incredibly painfull to test
* Do not rely on conversions between float and extended floating point types (#2046) The issue we have is that our tests rely extensively on those conversions which makes it incredibly painfull to test * Fix including `<complex>` when bad CUDA bfloat/half macros are used. (#2226) * Add <complex> test for bad macros being defined * Fix <complex> failing upon inclusion when bad macros are defined * Rather use explicit specializations and some evil hackery to get the complex interop to work * Fix typos * Inline everything * Move workarounds together * Use conversion functions instead of explicit specializations * Drop unneeded conversions --------- Co-authored-by: Michael Schellenberger Costa <[email protected]> --------- Co-authored-by: Michael Schellenberger Costa <[email protected]>
The issue we have is that our tests rely extensively on those conversions which makes it incredibly painfull to test
Fixes nvbug4739601