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

Add support to __half and nv_bfloat16 to most math functions #1554

Merged
merged 12 commits into from
Jun 2, 2023

Conversation

Kh4ster
Copy link
Contributor

@Kh4ster Kh4ster commented May 30, 2023

This PR adds support to __half and nb_bfloat16 to most math functions:

  • cos
  • sin
  • exp
  • log
  • max
  • min
  • sqrt

@Kh4ster Kh4ster requested a review from a team as a code owner May 30, 2023 12:46
@github-actions github-actions bot added the cpp label May 30, 2023
Copy link
Contributor

@Nyrio Nyrio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. A few nitpicks

cpp/include/raft/core/detail/macros.hpp Outdated Show resolved Hide resolved
cpp/include/raft/core/math.hpp Outdated Show resolved Hide resolved
cpp/include/raft/core/math.hpp Outdated Show resolved Hide resolved
Comment on lines 120 to 128
#if (__CUDA_ARCH__ >= 530)
return ::hcos(x);
#else
// static_assert(false) would be evaluated during host compilation stage while __CUDA_ARCH__ is at
// device compilation stage Using this sizeof(T) != sizeof(T) makes it work as it's only triggered
// during template instantiation and thus at device compilation stage
static_assert(sizeof(T) != sizeof(T), "__half is only supported on __CUDA_ARCH__ >= 530");
return T{};
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question for @cjnolet : iirc RAPIDS is Pascal+, shall we have a centralized check of the minimum arch somewhere and remove these checks here, or would this have too many consequences on repos using raft?

cpp/include/raft/core/math.hpp Outdated Show resolved Hide resolved
@Kh4ster Kh4ster requested review from a team as code owners May 30, 2023 13:40
cpp/include/raft/core/math.hpp Outdated Show resolved Hide resolved
@Nyrio Nyrio added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 30, 2023
@Nyrio
Copy link
Contributor

Nyrio commented May 30, 2023

Note: this PR solves #1542

@github-actions github-actions bot removed the python label May 31, 2023
rapids-bot bot pushed a commit that referenced this pull request Jun 1, 2023
This PR fixes some outdated pinnings in `branch-23.08` and applies some fixes to `update-version.sh`.

See also:
rapidsai/cuml#5440
#1554 (comment)

Authors:
  - Bradley Dice (https://github.com/bdice)
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Ray Douglass (https://github.com/raydouglass)
  - Divye Gala (https://github.com/divyegala)
  - AJ Schmidt (https://github.com/ajschmidt8)

URL: #1556
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the PR!

@cjnolet
Copy link
Member

cjnolet commented Jun 2, 2023

/merge

@rapids-bot rapids-bot bot merged commit 6bc237f into rapidsai:branch-23.08 Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

4 participants