Skip to content

fix: support nvcc and test #2461

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Sep 10, 2020
Merged

fix: support nvcc and test #2461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
58c532f
fix: support nvcc and test
henryiii Sep 4, 2020
11088dd
fixup! fix: support nvcc and test
henryiii Sep 4, 2020
37ace8b
docs: mention what compilers fail
henryiii Sep 4, 2020
da1d2dc
fix: much simpler logic
henryiii Sep 5, 2020
424ea1a
refactor: slightly faster / clearer
henryiii Sep 10, 2020
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
22 changes: 22 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,28 @@ jobs:
- name: Interface test
run: cmake --build build --target test_cmake_build

cuda:
runs-on: ubuntu-latest
name: "🐍 3.8 • CUDA 11 • Ubuntu 20.04"
container: nvidia/cuda:11.0-devel-ubuntu20.04

steps:
- uses: actions/checkout@v2

# tzdata will try to ask for the timezone, so set the DEBIAN_FRONTEND
- name: Install 🐍 3
run: apt-get update && DEBIAN_FRONTEND="noninteractive" apt-get install -y cmake python3-dev python3-pytest

- name: Configure
run: cmake -S . -B build -DPYBIND11_CUDA_TESTS=ON -DPYBIND11_WERROR=ON -DDOWNLOAD_CATCH=ON

- name: Build
run: cmake --build build -j2 -v

- name: Python tests
run: cmake --build build --target pytest


install-classic:
name: "🐍 3.5 • Debian • x86 • Install"
runs-on: ubuntu-latest
Expand Down
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ In addition to the core functionality, pybind11 provides some extra goodies:
4. Intel C++ compiler 17 or newer (16 with pybind11 v2.0 and 15 with pybind11
v2.0 and a [workaround][intel-15-workaround])
5. Cygwin/GCC (tested on 2.5.1)
6. NVCC (CUDA 11 tested)

## About

Expand Down
9 changes: 5 additions & 4 deletions include/pybind11/cast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,7 @@ template <typename CharT> using is_std_char_type = any_of<
std::is_same<CharT, wchar_t> /* std::wstring */
>;


template <typename T>
struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value && !is_std_char_type<T>::value>> {
using _py_type_0 = conditional_t<sizeof(T) <= sizeof(long), long, long long>;
Expand Down Expand Up @@ -1034,12 +1035,12 @@ struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value && !is_std_char_t
: (py_type) PYBIND11_LONG_AS_LONGLONG(src.ptr());
}

// Python API reported an error
bool py_err = py_value == (py_type) -1 && PyErr_Occurred();

// Protect std::numeric_limits::min/max with parentheses
Copy link
Member

Choose a reason for hiding this comment

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

Not a big fan of this change, isn't there some way to #pragma be quiet NVCC?

Copy link
Collaborator Author

@henryiii henryiii Sep 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It's poorly documented, but yes, I think more recent NVCC versions support suppression if you can find the right code. But this will actually generate simpler/faster runtime code with 1-2 fewer checks, if this happens fairly often. We can move the sizeof check to compile time, too, I believe.

I also think the logic may be flawed; it might be good to rework with new logic. Also I'm surprised it's showing up with some very odd types; things like pointers were static_cast to an integer is not allowed (which is why it's using C-style casts); I guess this is used to covert pointer addresses to Python ints?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Check the new version out! Simpler logic (the tests here are pretty comprehensive, don't ask me how I know that 😊), with no more pointless comparisons.

if (py_err || (std::is_integral<T>::value && sizeof(py_type) != sizeof(T) &&
(py_value < (py_type) (std::numeric_limits<T>::min)() ||
py_value > (py_type) (std::numeric_limits<T>::max)()))) {
// Check to see if the conversion is valid (integers should match exactly)
// Signed/unsigned checks happen elsewhere
if (py_err || (std::is_integral<T>::value && sizeof(py_type) != sizeof(T) && py_value != (py_type) (T) py_value)) {
bool type_error = py_err && PyErr_ExceptionMatches(
#if PY_VERSION_HEX < 0x03000000 && !defined(PYPY_VERSION)
PyExc_SystemError
Expand Down
7 changes: 7 additions & 0 deletions include/pybind11/numpy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1483,7 +1483,14 @@ struct vectorize_arg {

template <typename Func, typename Return, typename... Args>
struct vectorize_helper {

// NVCC for some reason breaks if NVectorized is private
Copy link
Collaborator

Choose a reason for hiding this comment

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

What exactly breaks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It claims NVectorize is private in this context and quits.

[ 73%] Building CUDA object tests/CMakeFiles/pybind11_tests.dir/test_opaque_types.cpp.o
cd /build/tests && /usr/local/cuda/bin/nvcc  -Dpybind11_tests_EXPORTS -I/pybind11/include -isystem=/usr/include/python3.8  -O1 -DNDEBUG -Xcompiler=-fPIC -Xcompiler=-fvisibility=hidden   -Werror all-warnings -std=c++11 -x cu -c /pybind11/tests/test_opaque_types.cpp -o CMakeFiles/pybind11_tests.dir/test_opaque_types.cpp.o
/pybind11/tests/test_numpy_vectorize.cpp: In lambda function:
/pybind11/tests/test_numpy_vectorize.cpp:86:6: error: 'constexpr const size_t pybind11::detail::vectorize_helper<double (*)(int, float, double), double, int, float, double>::NVectorized' is private within this context
   86 |         std::array<py::buffer_info, 3> buffers {{ arg1.request(), arg2.request(), arg3.request() }};
      |      ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/pybind11/include/pybind11/numpy.h:1490:25: note: declared private here
 1490 |     static constexpr size_t NVectorized = constexpr_sum(vectorize_arg<Args>::vectorize...);

#ifdef __CUDACC__
public:
#else
private:
#endif

static constexpr size_t N = sizeof...(Args);
static constexpr size_t NVectorized = constexpr_sum(vectorize_arg<Args>::vectorize...);
static_assert(NVectorized >= 1,
Expand Down
43 changes: 38 additions & 5 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/../tools")

option(PYBIND11_WERROR "Report all warnings as errors" OFF)
option(DOWNLOAD_EIGEN "Download EIGEN (requires CMake 3.11+)" OFF)
option(PYBIND11_CUDA_TESTS "Enable building CUDA tests (requires CMake 3.12+)" OFF)
set(PYBIND11_TEST_OVERRIDE
""
CACHE STRING "Tests from ;-separated list of *.cpp files will be built instead of all tests")
Expand All @@ -49,6 +50,14 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
"RelWithDebInfo")
endif()

if(PYBIND11_CUDA_TESTS)
enable_language(CUDA)
if(DEFINED CMAKE_CXX_STANDARD)
set(CMAKE_CUDA_STANDARD ${CMAKE_CXX_STANDARD})
endif()
set(CMAKE_CUDA_STANDARD_REQUIRED ON)
endif()

# Full set of test files (you can override these; see below)
set(PYBIND11_TEST_FILES
test_async.cpp
Expand Down Expand Up @@ -104,6 +113,16 @@ if((PYBIND11_TEST_FILES_ASYNC_I GREATER -1) AND (PYTHON_VERSION VERSION_LESS 3.5
list(REMOVE_AT PYBIND11_TEST_FILES ${PYBIND11_TEST_FILES_ASYNC_I})
endif()

# Skip tests for CUDA check:
# /pybind11/tests/test_constants_and_functions.cpp(125):
# error: incompatible exception specifications
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does "incompatible exception specification" mean?

Copy link
Collaborator Author

@henryiii henryiii Sep 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

That's printed by NVCC when running. It doesn't like either noexcept(false) or throw(), I believe, but the error message line is off (I think), so it's hard to tell what it is unhappy with.

list(FIND PYBIND11_TEST_FILES test_constants_and_functions.cpp PYBIND11_TEST_FILES_CAF_I)
if((PYBIND11_TEST_FILES_CAF_I GREATER -1) AND PYBIND11_CUDA_TESTS)
message(
STATUS "Skipping test_constants_and_functions due to incompatible exception specifications")
list(REMOVE_AT PYBIND11_TEST_FILES ${PYBIND11_TEST_FILES_CAF_I})
endif()

string(REPLACE ".cpp" ".py" PYBIND11_PYTEST_FILES "${PYBIND11_TEST_FILES}")

# Contains the set of test files that require pybind11_cross_module_tests to be
Expand Down Expand Up @@ -195,14 +214,16 @@ endif()
function(pybind11_enable_warnings target_name)
if(MSVC)
target_compile_options(${target_name} PRIVATE /W4)
elseif(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Intel|Clang)")
elseif(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Intel|Clang)" AND NOT PYBIND11_CUDA_TESTS)
target_compile_options(${target_name} PRIVATE -Wall -Wextra -Wconversion -Wcast-qual
-Wdeprecated)
endif()

if(PYBIND11_WERROR)
if(MSVC)
target_compile_options(${target_name} PRIVATE /WX)
elseif(PYBIND11_CUDA_TESTS)
target_compile_options(${target_name} PRIVATE "SHELL:-Werror all-warnings")
elseif(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Intel|Clang)")
target_compile_options(${target_name} PRIVATE -Werror)
endif()
Expand Down Expand Up @@ -239,12 +260,22 @@ foreach(t ${PYBIND11_CROSS_MODULE_GIL_TESTS})
endif()
endforeach()

# Support CUDA testing by forcing the target file to compile with NVCC
if(PYBIND11_CUDA_TESTS)
set_property(SOURCE ${PYBIND11_TEST_FILES} PROPERTY LANGUAGE CUDA)
endif()

foreach(target ${test_targets})
set(test_files ${PYBIND11_TEST_FILES})
if(NOT "${target}" STREQUAL "pybind11_tests")
set(test_files "")
endif()

# Support CUDA testing by forcing the target file to compile with NVCC
if(PYBIND11_CUDA_TESTS)
set_property(SOURCE ${target}.cpp PROPERTY LANGUAGE CUDA)
endif()

# Create the binding library
pybind11_add_module(${target} THIN_LTO ${target}.cpp ${test_files} ${PYBIND11_HEADERS})
pybind11_enable_warnings(${target})
Expand Down Expand Up @@ -354,8 +385,10 @@ add_custom_command(
$<TARGET_FILE:pybind11_tests>
${CMAKE_CURRENT_BINARY_DIR}/sosize-$<TARGET_FILE_NAME:pybind11_tests>.txt)

# Test embedding the interpreter. Provides the `cpptest` target.
add_subdirectory(test_embed)
if(NOT PYBIND11_CUDA_TESTS)
# Test embedding the interpreter. Provides the `cpptest` target.
add_subdirectory(test_embed)

# Test CMake build using functions and targets from subdirectory or installed location
add_subdirectory(test_cmake_build)
# Test CMake build using functions and targets from subdirectory or installed location
add_subdirectory(test_cmake_build)
endif()
4 changes: 3 additions & 1 deletion tests/test_constants_and_functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# -*- coding: utf-8 -*-
from pybind11_tests import constants_and_functions as m
import pytest

m = pytest.importorskip("pybind11_tests.constants_and_functions")


def test_constants():
Expand Down
16 changes: 11 additions & 5 deletions tests/test_copy_move.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,20 @@ TEST_SUBMODULE(copy_move_policies, m) {
m.attr("has_optional") = false;
#endif

// #70 compilation issue if operator new is not public
// #70 compilation issue if operator new is not public - simple body added
// but not needed on most compilers; MSVC and nvcc don't like a local
// struct not having a method defined when declared, since it can not be
// added later.
struct PrivateOpNew {
int value = 1;
private:
#if defined(_MSC_VER)
# pragma warning(disable: 4822) // warning C4822: local class member function does not have a body
#endif
void *operator new(size_t bytes);
void *operator new(size_t bytes) {
void *ptr = std::malloc(bytes);
if (ptr)
return ptr;
else
throw std::bad_alloc{};
}
};
py::class_<PrivateOpNew>(m, "PrivateOpNew").def_readonly("value", &PrivateOpNew::value);
m.def("private_op_new_value", []() { return PrivateOpNew(); });
Expand Down
4 changes: 2 additions & 2 deletions tests/test_virtual_functions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ class NCVirt {
std::string print_movable(int a, int b) { return get_movable(a, b).get_value(); }
};
class NCVirtTrampoline : public NCVirt {
#if !defined(__INTEL_COMPILER)
#if !defined(__INTEL_COMPILER) && !defined(__CUDACC__)
NonCopyable get_noncopyable(int a, int b) override {
PYBIND11_OVERLOAD(NonCopyable, NCVirt, get_noncopyable, a, b);
}
Expand Down Expand Up @@ -205,7 +205,7 @@ TEST_SUBMODULE(virtual_functions, m) {
.def(py::init<int, int>());

// test_move_support
#if !defined(__INTEL_COMPILER)
#if !defined(__INTEL_COMPILER) && !defined(__CUDACC__)
py::class_<NCVirt, NCVirtTrampoline>(m, "NCVirt")
.def(py::init<>())
.def("get_noncopyable", &NCVirt::get_noncopyable)
Expand Down