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

Quad-precision math #4098

Merged
merged 11 commits into from
Jun 22, 2021
Merged

Quad-precision math #4098

merged 11 commits into from
Jun 22, 2021

Conversation

dalg24
Copy link
Member

@dalg24 dalg24 commented Jun 16, 2021

Add support for GCC's 128-bit floating-point type

@mhoemmen
Copy link
Contributor

I did this for Trilinos many years ago (but post adding Kokkos to Tpetra, I think?).

cmake/kokkos_arch.cmake Outdated Show resolved Hide resolved
@@ -399,6 +399,8 @@ pipeline {
-DKokkos_ENABLE_TESTS=ON \
-DKokkos_ENABLE_OPENMP=ON \
-DKokkos_ENABLE_LIBDL=OFF \
-DKokkos_ENABLE_LIBQUADMATH=ON \
-DCMAKE_PREFIX_PATH=/usr/local/lib/gcc/x86_64-unknown-linux-gnu/5.3.0 \
Copy link
Member Author

@dalg24 dalg24 Jun 16, 2021

Choose a reason for hiding this comment

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

This is a bit frustrating. This is only needed for CMake, g++ does not need to add anything to the include path to find <quadmath.h>. Also -lquadmath on the link line would do.
Unfortunately I am not sure how I would do that with our CMake files, especially if it has to behave the same with Trilinos.

inline __float128 hypot(__float128 x, __float128 y) { return ::hypotq(x, y); }
// Exponential functions
inline __float128 exp(__float128 x) { return ::expq(x); }
#if defined(KOKKOS_COMPILER_GNU) && (KOKKOS_COMPILER_GNU >= 910)
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be checking the stdlibc++ version instead

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I am not sure... There does not seem to be any macro to check the libquadmath version.

@dalg24 dalg24 marked this pull request as ready for review June 18, 2021 14:35
Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

Looks OK to me (maybe apart from the CMAKE_PREFIX_PATH weirdness)

@crtrott crtrott merged commit 905b426 into kokkos:develop Jun 22, 2021
@ikalash
Copy link
Contributor

ikalash commented Jun 23, 2021

I did this for Trilinos many years ago (but post adding Kokkos to Tpetra, I think?).

Hi @mhoemmen ! hope you are doing well! You may be happy to hear I am reviving all the float128 stuff you put in awhile ago, as well as making extensions for 'long double' scalar types. Unfortunately there have been a number of issues getting the software stack to cooperate, sigh...

@ikalash
Copy link
Contributor

ikalash commented Jun 23, 2021

Thanks for doing this @dalg24 ! Would Kokkos automatically detect if quadmath is enabled in Trilnos when being built as a part of Trilinos and set the configure flags appropriately?

@dalg24
Copy link
Member Author

dalg24 commented Jun 23, 2021

Yes

IF(Trilinos_ENABLE_Kokkos AND TPL_ENABLE_quadmath)
SET(LIBQUADMATH_DEFAULT ON)
ELSE()
SET(LIBQUADMATH_DEFAULT OFF)
ENDIF()
KOKKOS_TPL_OPTION(LIBQUADMATH ${LIBQUADMATH_DEFAULT} TRIBITS quadmath)

@dalg24 dalg24 deleted the libquadmath branch June 23, 2021 16:57
@ikalash
Copy link
Contributor

ikalash commented Jun 23, 2021

Regarding adding the extended scalar types to Kokkos::rand - I understand the choice here. The problem is that Kokkos::rand is used within basic Trilinos routines with the implied scalar type, e.g., https:/trilinos/Trilinos/blob/master/packages/tpetra/core/src/Tpetra_MultiVector_def.hpp#L2345-L2350 . I could try to change this if Trilnos folks are not opposed. What is the best way to do this - do a downcase only in the case of a 'long double' or '__float128'?

@dalg24
Copy link
Member Author

dalg24 commented Jun 23, 2021

Use Kokkos::Experimental::finite_max<T>::value instead

@ikalash
Copy link
Contributor

ikalash commented Jun 23, 2021

Use Kokkos::Experimental::finite_max<T>::value instead

This is equivalent to Kokkos::rand?

@ikalash
Copy link
Contributor

ikalash commented Jun 23, 2021

Ah ok, never mind, I see what you are suggesting. Sorry, was looking in the wrong place. I'll try that.

@ikalash
Copy link
Contributor

ikalash commented Jun 23, 2021

So I get the following error when I switch to Kokkos::Experimental::finite_max<T>::value:

/nightlyCDash/repos/Trilinos/packages/tpetra/core/src/Tpetra_MultiVector_def.hpp:2350:60: error: ‘value’ is not a member of ‘Kokkos::Experimental::finite_max<Kokkos::complex<double> >’
 2350 |     const IST max = Kokkos::Experimental::finite_max<IST>::value;
      |                                                            ^~~~~

I guess there are not complex implementations of finite_max?

@dalg24
Copy link
Member Author

dalg24 commented Jun 23, 2021

No but you can refer to the discussion on #4033 and do something like 9a78864 where you'd return whatever you think is relevant for a complex number

@mhoemmen
Copy link
Contributor

mhoemmen commented Jun 23, 2021

@ikalash wrote:

hope you are doing well!

I am, thank you! Hope you are too! : - )

Unfortunately there have been a number of issues getting the software stack to cooperate, sigh...

Par for the course ; - )

You may be happy to hear I am reviving all the float128 stuff you put in awhile ago, as well as making extensions for long double scalar types.

Excellent! I do hope it works out! These days I generally avoid long double, because on Windows it's the same as double (alas). It could be interesting to work on a more portable solution, that defines a type alias promising 128-bit floating-point arithmetic. This would align with the P1467r4 C++ Standard Committee proposal.

@ikalash
Copy link
Contributor

ikalash commented Jun 23, 2021

Excellent! I do hope it works out! These days I generally avoid long double, because on Windows it's the same as double (alas). It could be interesting to work on a more portable solution, that defines a type alias promising 128-bit floating-point arithmetic. This would align with the P1467r4 C++ Standard Committee proposal.

Who uses Windows for scientific computing though?? (I know people do, I just always find it hard to believe). I have been lamenting that you are not around to discuss what I've been doing! It seems like there is a fair amount of interest in the extended scalar types within Trilinos, so it will be interesting to see if it goes anyway.

@mhoemmen
Copy link
Contributor

@ikalash wrote:

Who uses Windows for scientific computing though??

Ha! ; - P Y'all need to remember that a lot of computation happens on the desktop. Visual Studio's C++ support really took off in the past few years -- these days, we're using a lot of C++20 even without requiring the latest compiler version.

I've seen long double have 64 bits, 80 bits, or 128 bits, depending on the hardware and compiler. It's not very nice if you're trying to write portable code. One possible portable fall-back would be QD. Trilinos had QD support at one time, though it wasn't kept up. These days, it might be better to look into ReproBLAS techniques. @jjellio knows more and has done GPU performance studies.

I have been lamenting that you are not around to discuss what I've been doing!

Likewise! : - ) (I'd be happy to talk more offline about contract work through my employer if you're interested; if not, no worries!)

It seems like there is a fair amount of interest in the extended scalar types within Trilinos, so it will be interesting to see if it goes anyway.

Best wishes with that work!

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.

6 participants