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 mathematical constants #4519

Merged
merged 5 commits into from
Nov 22, 2021
Merged

Add mathematical constants #4519

merged 5 commits into from
Nov 22, 2021

Conversation

dalg24
Copy link
Member

@dalg24 dalg24 commented Nov 10, 2021

Fix #4507
Backporting C++20 facility https://eel.is/c++draft/numbers
Individual traits with a value member for each 13 constants defined in the standard.

@dalg24 dalg24 force-pushed the math_constants branch 2 times, most recently from 46b572e to 339e91e Compare November 10, 2021 22:40
@dalg24 dalg24 requested a review from nliber November 11, 2021 13:48
@dalg24
Copy link
Member Author

dalg24 commented Nov 12, 2021

Hold on before you merge that one. I probably should have used variable templates. I forgot they were available in C++14

@dalg24
Copy link
Member Author

dalg24 commented Nov 12, 2021

Except that inline variables only come with C++17...

@fnrizzi fnrizzi self-requested a review November 17, 2021 12:30
Copy link
Member

@crtrott crtrott left a comment

Choose a reason for hiding this comment

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

Discussed this with @dalg24 today there is a slightly different implementation idea in order to avoid introducing traits which are not part of the standard in the public Kokkos namespace (i.e. put the trait into Impl, and only the variable declaration in Kokkos)

#include <Kokkos_Core.hpp>

template <class T>
KOKKOS_FUNCTION T *take_address_of(T &arg) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious:: why not use std::addressof?

Copy link
Member Author

Choose a reason for hiding this comment

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

__host__ __device__ annotation missing

Copy link
Contributor

Choose a reason for hiding this comment

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

@crtrott crtrott merged commit f612cb8 into kokkos:develop Nov 22, 2021
@dalg24 dalg24 deleted the math_constants branch November 22, 2021 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants