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 compile errors with MSVC(alternative to #904) #908

Merged
merged 3 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
47 changes: 47 additions & 0 deletions .github/workflows/windows.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
name: github-windows

on: [push, pull_request]

concurrency:
group: ${ {github.event_name }}-${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: ${{github.event_name == 'pull_request'}}

jobs:
windows-threads:
name: Windows Threads
runs-on: windows-2022

steps:
- uses: actions/checkout@v3
- name: Install dependencies via vcpkg
uses: johnwason/vcpkg-action@v5
id: vcpkg
with:
pkgs: boost-test boost-program-options boost-geometry benchmark
triplet: x64-windows-release
token: ${{ github.token }}
- uses: actions/checkout@v3
with:
repository: kokkos/kokkos
ref: 4.0.00
path: ${GITHUB_WORKSPACE}/../kokkos
- name: Install Kokkos
shell: bash
working-directory: ${GITHUB_WORKSPACE}/../kokkos
run: |
mkdir build
cd build
cmake -D CMAKE_INSTALL_PREFIX=C:\kokkos-install \
-D Kokkos_ENABLE_THREADS=ON \
..
cmake --build . --target install -- -m
- name: Configure ArborX
run: |
mkdir build
cd build
cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS="/EHsc /bigobj" -DKokkos_ROOT="C:\kokkos-install" ${{ steps.vcpkg.outputs.vcpkg-cmake-config }} -DARBORX_ENABLE_MPI=OFF -DARBORX_ENABLE_TESTS=ON -DARBORX_ENABLE_EXAMPLES=ON -DARBORX_ENABLE_BENCHMARKS=ON ..
- name: Build ArborX
shell: bash
run: |
cmake --build build --target install -- -m
Copy link
Contributor

Choose a reason for hiding this comment

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

We build but we don't actually run anything. Is that an oversight?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No we have problems with boost at runtime.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just Boost.Test or also Boost.Program_options?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We had issue with both. They both need to be compiled

Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that #902 successfully ran tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that #902 successfully ran tests.

I don't understand that comment

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand that comment

#902 had

 - name: Build ArborX
      shell: bash
      run: |
        cmake --build build --target install -- -m
        cd build
        ctest -C Debug -V

and ran the tests (without enabling benchmarks).

cd build
11 changes: 8 additions & 3 deletions benchmarks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ find_package(benchmark 1.5.4 REQUIRED)
message(STATUS "Found benchmark: ${benchmark_DIR} (version \"${benchmark_VERSION}\")")

add_subdirectory(brute_force_vs_bvh)
add_subdirectory(bvh_driver)
add_subdirectory(dbscan)
add_subdirectory(develop)
add_subdirectory(execution_space_instances)
add_subdirectory(union_find)
if(NOT WIN32)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a FIXME with a suitable comment describing we this gets disabled here.

# FIXME: for now, skip the benchmarks using Google benchmark
# when building for Windows, as we have trouble linking it
# with the installed version of the Google benchmark
add_subdirectory(bvh_driver)
add_subdirectory(develop)
add_subdirectory(union_find)
endif()

if (ARBORX_ENABLE_MPI)
add_subdirectory(distributed_tree_driver)
Expand Down
5 changes: 3 additions & 2 deletions benchmarks/bvh_driver/benchmark_registration.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ makeSpatialQueries(int n_values, int n_queries, int n_neighbors,
n_queries);
// Radius is computed so that the number of results per query for a uniformly
// distributed points in a [-a,a]^3 box is approximately n_neighbors.
// Calculation: n_values*(4/3*M_PI*r^3)/(2a)^3 = n_neighbors
double const r = std::cbrt(static_cast<double>(n_neighbors) * 6. / M_PI);
// Calculation: n_values*(4/3*pi*r^3)/(2a)^3 = n_neighbors
double const r = std::cbrt(static_cast<double>(n_neighbors) * 6. /
aprokop marked this conversation as resolved.
Show resolved Hide resolved
Kokkos::numbers::pi_v<double>);
using ExecutionSpace = typename DeviceType::execution_space;
Kokkos::parallel_for(
"bvh_driver:setup_radius_search_queries",
Expand Down
14 changes: 10 additions & 4 deletions benchmarks/dbscan/converter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
#include <stdexcept>
#include <vector>

#ifdef _MSC_VER
#define ARBORX_STRTOK_R strtok_s
#else
#define ARBORX_STRTOK_R strtok_r
#endif

class Points
{
private:
Expand Down Expand Up @@ -197,15 +203,15 @@ auto loadTaxiPortoData(std::string const &filename)
char *end_str;
wordNo = 0;
lonlatno = 0;
pch = strtok_r(line, "\"[", &end_str);
pch = ARBORX_STRTOK_R(line, "\"[", &end_str);
while (pch != nullptr)
{
if (wordNo > 0)
{
char *pch2;
char *end_str2;

pch2 = strtok_r(pch, ",", &end_str2);
pch2 = ARBORX_STRTOK_R(pch, ",", &end_str2);

if (strcmp(pch2, "]") < 0 && lonlatno < 255)
{
Expand All @@ -218,7 +224,7 @@ auto loadTaxiPortoData(std::string const &filename)
{
longitudes.push_back(thisWord);
// printf("lon %f",thisWord);
pch2 = strtok_r(nullptr, ",", &end_str2);
pch2 = ARBORX_STRTOK_R(nullptr, ",", &end_str2);
thisWord = atof(pch2);
if (thisWord < 42 && thisWord > 40)
{
Expand All @@ -235,7 +241,7 @@ auto loadTaxiPortoData(std::string const &filename)
}
}
}
pch = strtok_r(nullptr, "[", &end_str);
pch = ARBORX_STRTOK_R(nullptr, "[", &end_str);
wordNo++;
}
// printf("num lonlat were %d x 2\n",lonlatno);
Expand Down
10 changes: 6 additions & 4 deletions benchmarks/distributed_tree_driver/distributed_tree_driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -460,13 +460,15 @@ int main_(std::vector<std::string> const &args, const MPI_Comm comm)
r = static_cast<double>(n_neighbors) - 1.;
break;
case 2:
// Derivation (first term): n_values*(M_PI*r^2)/(2a)^2 = n_neighbors
r = std::sqrt(static_cast<double>(n_neighbors) * 4. / M_PI) -
// Derivation (first term): n_values*(pi*r^2)/(2a)^2 = n_neighbors
r = std::sqrt(static_cast<double>(n_neighbors) * 4. /
Kokkos::numbers::pi_v<double>) -
(1. + std::sqrt(2.)) / 2;
break;
case 3:
// Derivation (first term): n_values*(4/3*M_PI*r^3)/(2a)^3 = n_neighbors
r = std::cbrt(static_cast<double>(n_neighbors) * 6. / M_PI) -
// Derivation (first term): n_values*(4/3*pi*r^3)/(2a)^3 = n_neighbors
r = std::cbrt(static_cast<double>(n_neighbors) * 6. /
Kokkos::numbers::pi_v<double>) -
(1. + std::cbrt(3.)) / 2;
break;
}
Expand Down
4 changes: 2 additions & 2 deletions examples/raytracing/example_raytracing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ int main(int argc, char *argv[])
b.minCorner()[2] +
Kokkos::rand<GeneratorType, float>::draw(g, dz)};

float upsilon =
Kokkos::rand<GeneratorType, float>::draw(g, 2.f * M_PI);
float upsilon = Kokkos::rand<GeneratorType, float>::draw(
g, 2.f * Kokkos::numbers::pi_v<float>);
float theta =
acos(1 - 2 * Kokkos::rand<GeneratorType, float>::draw(g));
ArborX::Experimental::Vector direction{
Expand Down
2 changes: 1 addition & 1 deletion src/ArborX_BruteForce.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class BruteForce

template <typename ExecutionSpace, typename Predicates,
typename CallbackOrView, typename View, typename... Args>
std::enable_if_t<Kokkos::is_view<std::decay_t<View>>{}>
std::enable_if_t<Kokkos::is_view_v<std::decay_t<View>>>
query(ExecutionSpace const &space, Predicates const &predicates,
CallbackOrView &&callback_or_view, View &&view, Args &&...args) const
{
Expand Down
4 changes: 2 additions & 2 deletions src/ArborX_LinearBVH.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class BasicBoundingVolumeHierarchy

template <typename ExecutionSpace, typename Predicates,
typename CallbackOrView, typename View, typename... Args>
std::enable_if_t<Kokkos::is_view<std::decay_t<View>>{}>
std::enable_if_t<Kokkos::is_view_v<std::decay_t<View>>>
query(ExecutionSpace const &space, Predicates const &predicates,
CallbackOrView &&callback_or_view, View &&view, Args &&...args) const
{
Expand Down Expand Up @@ -140,7 +140,7 @@ class BoundingVolumeHierarchy

template <typename ExecutionSpace, typename Predicates,
typename CallbackOrView, typename View, typename... Args>
std::enable_if_t<Kokkos::is_view<std::decay_t<View>>{}>
std::enable_if_t<Kokkos::is_view_v<std::decay_t<View>>>
query(ExecutionSpace const &space, Predicates const &predicates,
CallbackOrView &&callback_or_view, View &&view, Args &&...args) const
{
Expand Down
24 changes: 12 additions & 12 deletions src/details/ArborX_DetailsCrsGraphWrapperImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,8 @@ struct Iota

template <typename Tag, typename ExecutionSpace, typename Predicates,
typename OffsetView, typename OutView>
std::enable_if_t<std::is_same<Tag, SpatialPredicateTag>{} ||
std::is_same<Tag, Experimental::OrderedSpatialPredicateTag>{}>
std::enable_if_t<std::is_same_v<Tag, SpatialPredicateTag> ||
std::is_same_v<Tag, Experimental::OrderedSpatialPredicateTag>>
allocateAndInitializeStorage(Tag, ExecutionSpace const &space,
Predicates const &predicates, OffsetView &offset,
OutView &out, int buffer_size)
Expand All @@ -336,7 +336,7 @@ allocateAndInitializeStorage(Tag, ExecutionSpace const &space,

template <typename Tag, typename ExecutionSpace, typename Predicates,
typename OffsetView, typename OutView>
std::enable_if_t<std::is_same<Tag, NearestPredicateTag>{}>
std::enable_if_t<std::is_same_v<Tag, NearestPredicateTag>>
allocateAndInitializeStorage(Tag, ExecutionSpace const &space,
Predicates const &predicates, OffsetView &offset,
OutView &out, int /*buffer_size*/)
Expand All @@ -362,8 +362,8 @@ allocateAndInitializeStorage(Tag, ExecutionSpace const &space,
template <typename Tag, typename Tree, typename ExecutionSpace,
typename Predicates, typename OutputView, typename OffsetView,
typename Callback>
std::enable_if_t<!is_tagged_post_callback<Callback>{} &&
Kokkos::is_view<OutputView>{} && Kokkos::is_view<OffsetView>{}>
std::enable_if_t<!is_tagged_post_callback<Callback>::value &&
Kokkos::is_view_v<OutputView> && Kokkos::is_view_v<OffsetView>>
queryDispatch(Tag, Tree const &tree, ExecutionSpace const &space,
Predicates const &predicates, Callback const &callback,
OutputView &out, OffsetView &offset,
Expand Down Expand Up @@ -436,7 +436,7 @@ queryDispatch(Tag, Tree const &tree, ExecutionSpace const &space,

template <typename Tag, typename Tree, typename ExecutionSpace,
typename Predicates, typename Indices, typename Offset>
inline std::enable_if_t<Kokkos::is_view<Indices>{} && Kokkos::is_view<Offset>{}>
inline std::enable_if_t<Kokkos::is_view_v<Indices> && Kokkos::is_view_v<Offset>>
queryDispatch(Tag, Tree const &tree, ExecutionSpace const &space,
Predicates const &predicates, Indices &indices, Offset &offset,
Experimental::TraversalPolicy const &policy =
Expand All @@ -449,7 +449,7 @@ queryDispatch(Tag, Tree const &tree, ExecutionSpace const &space,
template <typename Tag, typename Tree, typename ExecutionSpace,
typename Predicates, typename OutputView, typename OffsetView,
typename Callback>
inline std::enable_if_t<is_tagged_post_callback<Callback>{}>
inline std::enable_if_t<is_tagged_post_callback<Callback>::value>
queryDispatch(Tag, Tree const &tree, ExecutionSpace const &space,
Predicates const &predicates, Callback const &callback,
OutputView &out, OffsetView &offset,
Expand All @@ -464,8 +464,8 @@ queryDispatch(Tag, Tree const &tree, ExecutionSpace const &space,
}

template <typename Callback, typename Predicates, typename OutputView>
std::enable_if_t<!Kokkos::is_view<Callback>{} &&
!is_tagged_post_callback<Callback>{}>
std::enable_if_t<!Kokkos::is_view_v<Callback> &&
!is_tagged_post_callback<Callback>::value>
check_valid_callback_if_first_argument_is_not_a_view(
Callback const &callback, Predicates const &predicates,
OutputView const &out)
Expand All @@ -474,8 +474,8 @@ check_valid_callback_if_first_argument_is_not_a_view(
}

template <typename Callback, typename Predicates, typename OutputView>
std::enable_if_t<!Kokkos::is_view<Callback>{} &&
is_tagged_post_callback<Callback>{}>
std::enable_if_t<!Kokkos::is_view_v<Callback> &&
is_tagged_post_callback<Callback>::value>
check_valid_callback_if_first_argument_is_not_a_view(Callback const &,
Predicates const &,
OutputView const &)
Expand All @@ -484,7 +484,7 @@ check_valid_callback_if_first_argument_is_not_a_view(Callback const &,
}

template <typename View, typename Predicates, typename OutputView>
std::enable_if_t<Kokkos::is_view<View>{}>
std::enable_if_t<Kokkos::is_view_v<View>>
check_valid_callback_if_first_argument_is_not_a_view(View const &,
Predicates const &,
OutputView const &)
Expand Down
7 changes: 4 additions & 3 deletions src/geometry/ArborX_Ray.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ struct Vector : private Point

template <typename Point1, typename Point2>
KOKKOS_INLINE_FUNCTION constexpr std::enable_if_t<
GeometryTraits::is_point<Point1>{} && GeometryTraits::is_point<Point2>{} &&
GeometryTraits::dimension<Point1>::value == 3 &&
GeometryTraits::dimension<Point2>::value == 3,
GeometryTraits::is_point<Point1>::value &&
GeometryTraits::is_point<Point2>::value &&
GeometryTraits::dimension_v<Point1> == 3 &&
GeometryTraits::dimension_v<Point2> == 3,
Vector>
makeVector(Point1 const &begin, Point2 const &end)
{
Expand Down
1 change: 1 addition & 0 deletions test/tstQueryTreeManufacturedSolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include <algorithm>
#include <iostream>
#include <numeric>
#include <random>
#include <tuple>

Expand Down
30 changes: 25 additions & 5 deletions test/tstRay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ BOOST_AUTO_TEST_CASE(intersects_box)
{ \
float t0; \
float t1; \
constexpr auto inf = KokkosExt::ArithmeticTraits::infinity<float>::value; \
BOOST_TEST(!ArborX::Experimental::intersection(ray, box, t0, t1)); \
BOOST_TEST((t0 == inf || t1 == -inf)); \
} while (false)
Expand All @@ -182,6 +181,12 @@ BOOST_AUTO_TEST_CASE(ray_box_intersection, *boost::unit_test::tolerance(1e-6f))
using ArborX::Box;
using ArborX::Experimental::Ray;

#ifdef _MSC_VER
auto const inf = KokkosExt::ArithmeticTraits::infinity<float>::value;
#else
constexpr auto inf = KokkosExt::ArithmeticTraits::infinity<float>::value;
#endif

constexpr Box unit_box{{0, 0, 0}, {1, 1, 1}};

auto const sqrtf_5 = std::sqrt(5.f);
Expand Down Expand Up @@ -250,9 +255,15 @@ BOOST_AUTO_TEST_CASE(ray_box_distance)
{
using ArborX::Box;
using ArborX::Experimental::Ray;

// use const instead of constexpr because MSVC shows error(error:expression
// must have a constant value)
constexpr Box unit_box{{0, 0, 0}, {1, 1, 1}};

#ifdef _MSC_VER
auto const inf = KokkosExt::ArithmeticTraits::infinity<float>::value;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should still be constexpr when using linux

#else
constexpr auto inf = KokkosExt::ArithmeticTraits::infinity<float>::value;
#endif

// clang-format off
// origin is within the box
Expand Down Expand Up @@ -347,7 +358,6 @@ BOOST_AUTO_TEST_CASE(overlap_distance_sphere,
{ \
float t0; \
float t1; \
constexpr auto inf = KokkosExt::ArithmeticTraits::infinity<float>::value; \
BOOST_TEST(!ArborX::Experimental::intersection(ray, sphere, t0, t1)); \
BOOST_TEST((t0 == inf && t1 == -inf)); \
} while (false)
Expand All @@ -359,10 +369,15 @@ BOOST_AUTO_TEST_CASE(ray_sphere_intersection,
using ArborX::Experimental::Ray;

constexpr Sphere unit_sphere{{0, 0, 0}, 1};

auto const sqrtf_3 = std::sqrt(3.f);
auto const sqrtf_2 = std::sqrt(2.f);

#ifdef _MSC_VER
auto const inf = KokkosExt::ArithmeticTraits::infinity<float>::value;
#else
constexpr auto inf = KokkosExt::ArithmeticTraits::infinity<float>::value;
#endif

// clang-format off
// hit center of the sphere
ARBORX_TEST_RAY_SPHERE_INTERSECTION((Ray{{-2, 0, 0}, {1, 0, 0}}), unit_sphere, 1.f, 3.f);
Expand Down Expand Up @@ -471,7 +486,6 @@ BOOST_AUTO_TEST_CASE(intersects_triangle)
{ \
float t0; \
float t1; \
constexpr auto inf = KokkosExt::ArithmeticTraits::infinity<float>::value; \
BOOST_TEST(!ArborX::Experimental::intersection(ray, triangle, t0, t1)); \
BOOST_TEST((t0 == inf && t1 == -inf)); \
} while (false)
Expand All @@ -483,6 +497,12 @@ BOOST_AUTO_TEST_CASE(ray_triangle_intersection,
using ArborX::ExperimentalHyperGeometry::Point;
using ArborX::ExperimentalHyperGeometry::Triangle;

#ifdef _MSC_VER
auto const inf = KokkosExt::ArithmeticTraits::infinity<float>::value;
#else
constexpr auto inf = KokkosExt::ArithmeticTraits::infinity<float>::value;
#endif

constexpr Triangle unit_triangle{Point{0, 0, 0}, Point{1, 0, 0},
Point{0, 1, 0}};
constexpr Triangle narrow_triangle{Point{0.5f, 0.5f, 0},
Expand Down