Skip to content
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

Fix reduce to match the documentation and use numeric limits #920

Closed
Tracked by #101
gevtushenko opened this issue Mar 11, 2023 · 1 comment · Fixed by #3863
Closed
Tracked by #101

Fix reduce to match the documentation and use numeric limits #920

gevtushenko opened this issue Mar 11, 2023 · 1 comment · Fixed by #3863
Assignees
Labels
breaking Breaking change

Comments

@gevtushenko
Copy link
Collaborator

In cub::DeviceReduce::{Arg,}{Max,Min} we guarantee to use std::numeric_limits<T>::{lowest,max} but what we actually use is cub::Traits<T>::{Lowest,Max}. This means that if a user specializes std::numeric_limits<T> for a custom type, there will still be a compilation error. Reproducer:

#include <cub/cub.cuh>
#include <limits>

struct A {
};

namespace std {
  // Specialize numeric_limits for A
  template <>
  struct numeric_limits<A> {
    static constexpr bool is_specialized = true;
    static constexpr A min() noexcept { return A{}; }
    static constexpr A max() noexcept { return A{}; }
    static constexpr A lowest() noexcept { return A{}; }
  };
}

int main() {
  A* in{};
  A* out{};

  std::size_t temp_storage_bytes{};
  cub::DeviceReduce::Max(nullptr, temp_storage_bytes, in, out, 0);
}

results in the following error:

/cub/device/device_reduce.cuh(761): error: class "cub::Traits<A>" has no member "Lowest"

Since this is broken, we should change the docs along with the code to use ::cuda::std::numeric_limits instead. The only thing that prevents us from doing so is the fact that ::cuda::std::numeric_limits doesn't support extended floating point types (__half, __nv_bfloat16). @wmaxey for visibility.

If this issue becomes critical, we'll have to wrap ::cuda::std::numeric_limits into our own trait that supports extended floating point types. Otherwise, I'd prefer to wait till libcu++ trait supports extended fp types. Extended fp support in libcu++, it it's turn, is blocked by bf16 not being redistributable according to the EULA license (issue).

@jarmak-nv jarmak-nv transferred this issue from NVIDIA/cub Nov 8, 2023
@bernhardmgruber
Copy link
Contributor

Here is the issue to support half and bfloat16 in numeric_limits: #3044

@bernhardmgruber bernhardmgruber self-assigned this Feb 15, 2025
@bernhardmgruber bernhardmgruber added the breaking Breaking change label Feb 15, 2025
bernhardmgruber added a commit to bernhardmgruber/cccl that referenced this issue Feb 20, 2025
* Replace all uses of cub::Traits other than radix sort key twiddling by numeric_limits
* Drop obsolete specializations of cub::NumericTraits
* Fix radix sort custom type example mentioning non-existent cub::RadixTraits
* Replace cub::BaseTraits and cub::Traits by aliases so uses can no longer specialize it
* Deprecate cub::Traits::Max|Lowest
* Extend documentation of trait classes

Fixes: NVIDIA#920
@cccl-authenticator-app cccl-authenticator-app bot moved this to In Review in CCCL Feb 20, 2025
bernhardmgruber added a commit to bernhardmgruber/cccl that referenced this issue Feb 20, 2025
* Replace all uses of cub::Traits other than radix sort key twiddling by numeric_limits
* Drop obsolete specializations of cub::NumericTraits
* Fix radix sort custom type example mentioning non-existent cub::RadixTraits
* Replace cub::BaseTraits and cub::Traits by aliases so uses can no longer specialize it
* Deprecate cub::Traits::Max|Lowest
* Extend documentation of trait classes

Fixes: NVIDIA#920
bernhardmgruber added a commit to bernhardmgruber/cccl that referenced this issue Feb 20, 2025
* Replace all uses of cub::Traits other than radix sort key twiddling by numeric_limits
* Drop obsolete specializations of cub::NumericTraits
* Fix radix sort custom type example mentioning non-existent cub::RadixTraits
* Replace cub::BaseTraits and cub::Traits by aliases so uses can no longer specialize it
* Deprecate cub::Traits::Max|Lowest
* Extend documentation of trait classes

Fixes: NVIDIA#920
bernhardmgruber added a commit to bernhardmgruber/cccl that referenced this issue Feb 20, 2025
* Replace all uses of cub::Traits other than radix sort key twiddling by numeric_limits
* Drop obsolete specializations of cub::NumericTraits
* Fix radix sort custom type example mentioning non-existent cub::RadixTraits
* Replace cub::BaseTraits and cub::Traits by aliases so uses can no longer specialize it
* Deprecate cub::Traits::Max|Lowest
* Extend documentation of trait classes

Fixes: NVIDIA#920
bernhardmgruber added a commit to bernhardmgruber/cccl that referenced this issue Feb 20, 2025
* Replace all uses of cub::Traits other than radix sort key twiddling by numeric_limits
* Drop obsolete specializations of cub::NumericTraits
* Fix radix sort custom type example mentioning non-existent cub::RadixTraits
* Replace cub::BaseTraits and cub::Traits by aliases so uses can no longer specialize it
* Deprecate cub::Traits::Max|Lowest
* Extend documentation of trait classes

Fixes: NVIDIA#920
bernhardmgruber added a commit to bernhardmgruber/cccl that referenced this issue Feb 20, 2025
* Replace all uses of cub::Traits other than radix sort key twiddling by numeric_limits
* Drop obsolete specializations of cub::NumericTraits
* Fix radix sort custom type example mentioning non-existent cub::RadixTraits
* Replace cub::BaseTraits and cub::Traits by aliases so uses can no longer specialize it
* Deprecate cub::Traits::Max|Lowest
* Extend documentation of trait classes

Fixes: NVIDIA#920
bernhardmgruber added a commit to bernhardmgruber/cccl that referenced this issue Feb 21, 2025
* Replace all uses of cub::Traits other than radix sort key twiddling by numeric_limits
* Drop obsolete specializations of cub::NumericTraits
* Fix radix sort custom type example mentioning non-existent cub::RadixTraits
* Replace cub::BaseTraits and cub::Traits by aliases so uses can no longer specialize it
* Deprecate cub::Traits::Max|Lowest
* Extend documentation of trait classes

Fixes: NVIDIA#920
bernhardmgruber added a commit to bernhardmgruber/cccl that referenced this issue Feb 21, 2025
* Replace all uses of cub::Traits other than radix sort key twiddling by numeric_limits
* Drop obsolete specializations of cub::NumericTraits
* Fix radix sort custom type example mentioning non-existent cub::RadixTraits
* Replace cub::BaseTraits and cub::Traits by aliases so uses can no longer specialize it
* Deprecate cub::Traits::Max|Lowest
* Extend documentation of trait classes

Fixes: NVIDIA#920
@github-project-automation github-project-automation bot moved this from In Review to Done in CCCL Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants