-
Notifications
You must be signed in to change notification settings - Fork 928
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
More minor fixes for CCCL #17793
base: branch-25.04
Are you sure you want to change the base?
More minor fixes for CCCL #17793
Conversation
// types are returned as is. | ||
template <typename T, bool has_nulls> | ||
struct possibly_null_value; | ||
|
||
template <typename T> | ||
struct possibly_null_value<T, true> { | ||
using type = thrust::optional<T>; | ||
using type = cuda::std::optional<T>; |
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.
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.
Here, I might have answered it myself. #15091 (comment)
Device uses of
thrust::optional
can be replaced withcuda::std::optional
in a future PR, once we upgrade to CCCL 2.3 or newer
Is that why?
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.
Nope. We saw test failures with cuda::std::optional
here. #15091 (comment)
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.
Same failures appear again, there is definitely something fishy going on
This avoids failures in CCCL CI Pulled out of #17793 Authors: - Michael Schellenberger Costa (https://github.com/miscco) - Bradley Dice (https://github.com/bdice) Approvers: - Bradley Dice (https://github.com/bdice) - David Wendt (https://github.com/davidwendt) - James Lamb (https://github.com/jameslamb) URL: #17911
I updated this PR now that #17911 is in. It's down to just the AST optional that doesn't seem to work if it's changed from |
This drops the final use of
thrust::optional
in cuDF