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

Task/rhornung67/device numeric limits #1196

Merged
merged 18 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
87c4d77
Add "wrapper" capability to access std::numeric_limits functionality
rhornung67 Oct 4, 2023
9ad3a06
add basic tests for numeric_limits host and device operations
rhornung67 Oct 4, 2023
b255612
Add some comments to tests
rhornung67 Oct 4, 2023
f401e6e
Run clang format
rhornung67 Oct 5, 2023
90aed51
Convert core tests to use axom::numeric_limits
rhornung67 Oct 5, 2023
ec4e42f
Revert type change
rhornung67 Oct 5, 2023
7aadf4b
Experiment with test for device code
rhornung67 Oct 5, 2023
29f555f
Merge branch 'develop' into task/rhornung67/device-numeric-limits
rhornung67 Jul 11, 2024
ef7af10
Apply suggestions from code review
rhornung67 Jul 11, 2024
088e566
Merge branch 'develop' into task/rhornung67/device-numeric-limits
rhornung67 Jul 15, 2024
ab3106a
Convert all use of std::numeric_limits to axom::numeric_limits
rhornung67 Jul 15, 2024
7d4ca50
Merge branch 'task/rhornung67/device-numeric-limits' of github.com:LL…
rhornung67 Jul 15, 2024
2fea1ad
Merge branch 'develop' into task/rhornung67/device-numeric-limits
rhornung67 Jul 15, 2024
1ee4b00
Merge branch 'develop' into task/rhornung67/device-numeric-limits
rhornung67 Jul 16, 2024
07d7016
Merge branch 'develop' into task/rhornung67/device-numeric-limits
rhornung67 Jul 24, 2024
fc43024
Merge branch 'develop' into task/rhornung67/device-numeric-limits
rhornung67 Aug 5, 2024
effc432
Merge branch 'develop' into task/rhornung67/device-numeric-limits
rhornung67 Aug 9, 2024
e2602cb
Merge branch 'develop' into task/rhornung67/device-numeric-limits
rhornung67 Aug 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/axom/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ set(core_headers
IteratorBase.hpp
Macros.hpp
Map.hpp
NumericLimits.hpp
Path.hpp
StackArray.hpp
Types.hpp
Expand Down
38 changes: 38 additions & 0 deletions src/axom/core/NumericLimits.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) 2017-2023, Lawrence Livermore National Security, LLC and
// other Axom Project Developers. See the top-level LICENSE file for details.
//
// SPDX-License-Identifier: (BSD-3-Clause)

/*!
*
* \file NumericLimits.hpp
*
* \brief Header file containing portability layer for std::numeric_limits
* capabilities
*
*/

#ifndef AXOM_NUMERICLIMITS_HPP_
#define AXOM_NUMERICLIMITS_HPP_

#include "axom/config.hpp" // for compile-time definitions

#include <limits>

#if defined(AXOM_USE_CUDA)
#include <cuda/std/limits>
#endif

namespace axom
{
#if defined(AXOM_USE_CUDA) && defined(AXOM_DEVICE_CODE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to remove if defined(AXOM_DEVICE_CODE) and just have the AXOM_USE_CUDA guard? It's my impression that cuda::std should work on both the host and device.

Copy link
Member Author

Choose a reason for hiding this comment

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

If it does, that would be preferable. I need to look into that. @kennyweiss discussed this PR yesterday. That resulted in some concerns about several things in the code that we should discuss as a team. Unfortunately, that will have to wait for a couple of weeks as next Monday is a LLNL holiday and NECDC is the week after that.

Copy link
Member Author

@rhornung67 rhornung67 Oct 6, 2023

Choose a reason for hiding this comment

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

@publixsubfan cuda::std does work in device and host code. I ran into some issues with some Axom tests where cuda::std::numeric_limits does not support long double. The intent of my change was to use std::numeric_limits in host code for all builds. However, it's not clear to me that we need to support long double. long double is automatically converted to double in device code and attempting to pass long double data between host and device code is problematic.

template <typename T>
using numeric_limits = cuda::std::numeric_limits<T>;
Copy link
Member

Choose a reason for hiding this comment

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

Is the point of using CUDA-specific numeric limits to get host device on them? Or, does that not matter since they are probably all constexpr? Does HIP have a header like this? Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

The main point of this is to prevent compiler warnings (calling host function from host-device function) that a few users have reported. It is not needed for HIP since the amdclang compiler is a unified host-device compiler and know how to sort everything out.

#else
template <typename T>
using numeric_limits = std::numeric_limits<T>;
#endif

} // namespace axom

#endif // AXOM_NUMERICLIMITS_HPP_
1 change: 1 addition & 0 deletions src/axom/core/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ set(core_serial_tests
core_execution_space.hpp
core_map.hpp
core_memory_management.hpp
core_numeric_limits.hpp
core_Path.hpp
core_stack_array.hpp

Expand Down
3 changes: 2 additions & 1 deletion src/axom/core/tests/core_bit_utilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "axom/config.hpp"
#include "axom/core/Types.hpp"
#include "axom/core/NumericLimits.hpp"
#include "axom/core/utilities/Utilities.hpp"
#include "axom/core/utilities/BitUtilities.hpp"

Expand All @@ -22,7 +23,7 @@ T random_int()
{
static_assert(std::is_integral<T>::value, "T must be an integral type");

constexpr T max_int = std::numeric_limits<T>::max();
constexpr T max_int = axom::numeric_limits<T>::max();
constexpr double max_d = static_cast<double>(max_int);

const auto val = axom::utilities::random_real(0., max_d);
Expand Down
170 changes: 170 additions & 0 deletions src/axom/core/tests/core_numeric_limits.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
// Copyright (c) 2017-2023, Lawrence Livermore National Security, LLC and
// other Axom Project Developers. See the top-level LICENSE file for details.
//
// SPDX-License-Identifier: (BSD-3-Clause)

#include "axom/config.hpp" // for compile time definitions

#include "axom/core/NumericLimits.hpp"

// for gtest macros
#include "gtest/gtest.h"

//------------------------------------------------------------------------------
// UNIT TESTS
//------------------------------------------------------------------------------

//------------------------------------------------------------------------------
TEST(core_NumericLimits, check_CPU)
{
//
// Tests to compare axom::numeric_limits to std::numeric_limits
// to ensure that Axom type aliasing is correct.
//
EXPECT_TRUE(axom::numeric_limits<int>::lowest() ==
std::numeric_limits<int>::lowest());
EXPECT_TRUE(axom::numeric_limits<int>::min() == std::numeric_limits<int>::min());
EXPECT_TRUE(axom::numeric_limits<int>::max() == std::numeric_limits<int>::max());
EXPECT_TRUE(axom::numeric_limits<int>::is_signed ==
std::numeric_limits<int>::is_signed);

EXPECT_TRUE(axom::numeric_limits<float>::lowest() ==
std::numeric_limits<float>::lowest());
EXPECT_TRUE(axom::numeric_limits<float>::min() ==
std::numeric_limits<float>::min());
EXPECT_TRUE(axom::numeric_limits<float>::max() ==
std::numeric_limits<float>::max());

EXPECT_TRUE(axom::numeric_limits<double>::lowest() ==
std::numeric_limits<double>::lowest());
EXPECT_TRUE(axom::numeric_limits<double>::min() ==
std::numeric_limits<double>::min());
EXPECT_TRUE(axom::numeric_limits<double>::max() ==
std::numeric_limits<double>::max());
}

//------------------------------------------------------------------------------
#if defined(AXOM_USE_CUDA)
//
// Tests to ensure axom::numeric_limits type alias does the correct thing
// in host and CUDA device code.
//

//
// Simple device kernel
//
__global__ void cuda_kernel(int* a, size_t* b, float* c, double* d)
{
a[0] = axom::numeric_limits<int>::min();
b[0] = axom::numeric_limits<size_t>::max();
c[0] = axom::numeric_limits<float>::lowest();
d[0] = axom::numeric_limits<double>::max();
}

TEST(core_NumericLimits, check_CUDA)
{
//
// Device memory allocation and initialiation for a few different types.
//
int* a;
(void)cudaMalloc(&a, sizeof(int));
(void)cudaMemset(a, 0, sizeof(int));

size_t* b;
(void)cudaMalloc(&b, sizeof(size_t));
(void)cudaMemset(b, 0, sizeof(size_t));

float* c;
(void)cudaMalloc(&c, sizeof(float));
(void)cudaMemset(c, 0, sizeof(float));

double* d;
(void)cudaMalloc(&d, sizeof(double));
(void)cudaMemset(d, 0, sizeof(double));

//
// Set values in device code.
//
cuda_kernel<<<1, 1>>>(a, b, c, d);

//
// Copy device values back to host and compare with expectations....
//
int ha;
size_t hb;
float hc;
double hd;
(void)cudaMemcpy(&ha, a, sizeof(int), cudaMemcpyDeviceToHost);
(void)cudaMemcpy(&hb, b, sizeof(size_t), cudaMemcpyDeviceToHost);
(void)cudaMemcpy(&hc, c, sizeof(float), cudaMemcpyDeviceToHost);
(void)cudaMemcpy(&hd, d, sizeof(double), cudaMemcpyDeviceToHost);

EXPECT_TRUE(ha == axom::numeric_limits<int>::min());
EXPECT_TRUE(hb == axom::numeric_limits<size_t>::max());
EXPECT_TRUE(hc == axom::numeric_limits<float>::lowest());
EXPECT_TRUE(hd == axom::numeric_limits<double>::max());
}
#endif

//------------------------------------------------------------------------------
#if defined(AXOM_USE_HIP)
//
// Tests to ensure axom::numeric_limits type alias does the correct thing
// in host and CUDA device code.
//

//
// Simple device kernel
//
__global__ void hip_kernel(int* a, size_t* b, float* c, double* d)
{
a[0] = axom::numeric_limits<int>::min();
b[0] = axom::numeric_limits<size_t>::max();
c[0] = axom::numeric_limits<float>::lowest();
d[0] = axom::numeric_limits<double>::max();
}

TEST(core_NumericLimits, check_HIP)
{
//
// Device memory allocation and initialiation for a few different types.
//
int* a;
(void)hipMalloc(&a, sizeof(int));
(void)hipMemset(a, 0, sizeof(int));

size_t* b;
(void)hipMalloc(&b, sizeof(size_t));
(void)hipMemset(b, 0, sizeof(size_t));

float* c;
(void)hipMalloc(&c, sizeof(float));
(void)hipMemset(c, 0, sizeof(float));

double* d;
(void)hipMalloc(&d, sizeof(double));
(void)hipMemset(d, 0, sizeof(double));

//
// Set values in device code.
//
hip_kernel<<<1, 1>>>(a, b, c, d);

//
// Copy device values back to host and compare with expectations....
//
int ha;
size_t hb;
float hc;
double hd;
(void)hipMemcpy(&ha, a, sizeof(int), hipMemcpyDeviceToHost);
(void)hipMemcpy(&hb, b, sizeof(size_t), hipMemcpyDeviceToHost);
(void)hipMemcpy(&hc, c, sizeof(float), hipMemcpyDeviceToHost);
(void)hipMemcpy(&hd, d, sizeof(double), hipMemcpyDeviceToHost);

EXPECT_TRUE(ha == axom::numeric_limits<int>::min());
EXPECT_TRUE(hb == axom::numeric_limits<size_t>::max());
EXPECT_TRUE(hc == axom::numeric_limits<float>::lowest());
EXPECT_TRUE(hd == axom::numeric_limits<double>::max());
}
#endif
1 change: 1 addition & 0 deletions src/axom/core/tests/core_serial_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "core_execution_space.hpp"
#include "core_map.hpp"
#include "core_memory_management.hpp"
#include "core_numeric_limits.hpp"
#include "core_Path.hpp"
#include "core_stack_array.hpp"

Expand Down
10 changes: 5 additions & 5 deletions src/axom/core/tests/core_types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
#include "axom/config.hpp"
#include "axom/core/Types.hpp"
#include "axom/core/Macros.hpp"
#include "axom/core/NumericLimits.hpp"

// gtest includes
#include "gtest/gtest.h"

// C/C++ includes
#include <limits> // for std::numeric_limits
#include <type_traits> // for std::is_same, std::is_integral, etc.

#ifndef AXOM_USE_MPI
Expand Down Expand Up @@ -62,7 +62,7 @@ void check_real_type(std::size_t expected_num_bytes,
MPI_Datatype expected_mpi_type)
{
EXPECT_TRUE(std::is_floating_point<RealType>::value);
EXPECT_TRUE(std::numeric_limits<RealType>::is_signed);
EXPECT_TRUE(axom::numeric_limits<RealType>::is_signed);
EXPECT_EQ(sizeof(RealType), expected_num_bytes);

check_mpi_type<RealType>(expected_num_bytes, expected_mpi_type);
Expand All @@ -75,9 +75,9 @@ void check_integral_type(std::size_t expected_num_bytes,
int expected_num_digits,
MPI_Datatype expected_mpi_type)
{
EXPECT_TRUE(std::numeric_limits<IntegralType>::is_integer);
EXPECT_EQ(std::numeric_limits<IntegralType>::is_signed, is_signed);
EXPECT_EQ(std::numeric_limits<IntegralType>::digits, expected_num_digits);
EXPECT_TRUE(axom::numeric_limits<IntegralType>::is_integer);
EXPECT_EQ(axom::numeric_limits<IntegralType>::is_signed, is_signed);
EXPECT_EQ(axom::numeric_limits<IntegralType>::digits, expected_num_digits);
EXPECT_EQ(sizeof(IntegralType), expected_num_bytes);

check_mpi_type<IntegralType>(expected_num_bytes, expected_mpi_type);
Expand Down
2 changes: 2 additions & 0 deletions src/axom/core/tests/numerics_floating_point_limits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,7 @@ TEST(numerics_floating_point_limits, consistency_with_standard_numeric_limits)
{
check_type_limits<float>("float");
check_type_limits<double>("double");
#if !defined(AXOM_DEVICE_CODE)
check_type_limits<long double>("long double");
#endif
}