Refactor thrust::complex as a struct derived from cuda::std::complex
#454
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
There are some notable differences though. thrust::complex has been a bit more lenient when determining the type of arithmetic operations. That said, I believe being more strict is actually a feature not a bug
This commit refactors the thrust::complex type to be a struct derived from cuda::std::complex, enabling reuse of existing implementation logic. However, to maintain backward compatibility, certain operators are reintroduced to allow type promotion between 'float' and 'double' for the underlying type.
Blonck
requested review from
wmaxey and
griwes
and removed request for
a team
jrhemstad
reviewed
Sep 18, 2023
|
/ok to test |
|
/ok to test |
|
So this is failing because I removed the long double overloads that are actually removed and static asserted that we should never use them. I believe we should also remove that test |
|
/ok to test |
|
/ok to test |
miscco
approved these changes
Sep 22, 2023
miscco
reviewed
Sep 22, 2023
| @@ -1191,6 +1188,7 @@ arg(const complex<_Tp>& __c) | |||
| return _CUDA_VSTD::atan2(__c.imag(), __c.real()); | |||
| } | |||
|
|
|||
| #ifdef _LIBCUDACXX_HAS_COMPLEX_LONG_DOUBLE | |||
There was a problem hiding this comment.
@griwes This was preexisting in libcu++
I consider this a preexisting bug. Do you have any objections in disabling long double when it is not defined?
miscco
reviewed
Sep 22, 2023
|
/ok to test |
|
The complex files were under an old apache license we should clean the new files up given that we completely reimplemented it |
|
/ok to test |
miscco
added a commit
to miscco/cccl
that referenced
this pull request
Jan 16, 2024
jrhemstad
added a commit
that referenced
this pull request
Jan 17, 2024
…d::complex` (#454)" (#1286) Co-authored-by: Jake Hemstad <[email protected]>
miscco
added a commit
to miscco/cccl
that referenced
this pull request
Feb 22, 2024
…d::complex` (NVIDIA#454)" (NVIDIA#1286) We did not have any time to work on fixing the numerical issues with `cuda::std::complex` so punt on it for 2.4.x Co-authored-by: Jake Hemstad <[email protected]>
miscco
added a commit
to miscco/cccl
that referenced
this pull request
Mar 6, 2024
…d::complex` (NVIDIA#454)" This reverts commit 1f6e4bc. We face numerical issues with our cuda::std::complex implementation but did not have the capacity to work on it
miscco
added a commit
to miscco/cccl
that referenced
this pull request
Mar 6, 2024
…d::complex` (NVIDIA#454)" This reverts commit 1f6e4bc. We face numerical issues with our cuda::std::complex implementation but did not have the capacity to work on it
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.
This commit refactors the thrust::complex type to be a struct derived
from
cuda::std::complex, enabling reuse of existing implementationlogic. However, to maintain backward compatibility, certain
operators are reintroduced to allow type promotion between
floatand
doublefor the underlying type.This PR is derived from #260 and has been rebased onto the current main branch
Checklist