Skip to content

Commit

Permalink
Fix build issues related to libcudf/cudf changes (#322)
Browse files Browse the repository at this point in the history
Due to libcudf splitting into multiple shared libraries to fix debug builds, libcuspatial was left in a broken state. We don't fully understand why, but it seems like some combination of the linker pruning the `libcudf.so` dependency since it finds no symbols it uses in it BEFORE it adds `DT_NEEDED` entries from `libcudf.so` where it does find symbols.

Added some linker flags to temporarily fix the build while we continue investigating a more proper solution from the libcudf side.

Also fixed a minor gcc9 issue related to a copy elision.

Also fixed usage of a removed cudf Python API.
  • Loading branch information
Keith Kraus authored Nov 11, 2020
1 parent a066305 commit 6c2b52b
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- PR #294 Fix include of deprecated RMM header file.
- PR #296 Updates for RMM being header only.
- PR #298 Fix Python docs to render first argument of each public function.
- PR #322 Fix build issues related to libcudf split build changes


# cuSpatial 0.15.0 (26 Aug 2020)
Expand Down
14 changes: 5 additions & 9 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,6 @@ endif (CUDF_INCLUDE AND CUDF_LIBRARY)

find_package(GDAL REQUIRED)

message(STATUS "GDAL: GDAL_LIBRARIES set to ${GDAL_LIBRARIES}")
message(STATUS "GDAL: GDAL_INCLUDE_DIRS set to ${GDAL_INCLUDE_DIRS}")

if(NOT GDAL_FOUND)
message(FATAL_ERROR "GDAL not found, please check your settings.")
endif(NOT GDAL_FOUND)
Expand Down Expand Up @@ -254,7 +251,6 @@ endif(CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES)
include_directories("${CMAKE_BINARY_DIR}/include"
"${CMAKE_SOURCE_DIR}/include"
"${CMAKE_SOURCE_DIR}/src"
"${GDAL_INCLUDE_DIRS}"
"${RMM_INCLUDE}"
"${CUDF_INCLUDE}")

Expand All @@ -268,9 +264,7 @@ endif(CONDA_INCLUDE_DIRS)
link_directories("${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}" # CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES is an undocumented/unsupported variable containing the link directories for nvcc
"${CMAKE_BINARY_DIR}/lib"
"${FLATBUFFERS_LIBRARY_DIR}"
"${GDAL_LIBRARIES}"
"${GTEST_LIBRARY_DIR}"
"${CUDF_LIBRARY}")
"${GTEST_LIBRARY_DIR}")

if(CONDA_LINK_DIRS)
link_directories("${CONDA_LINK_DIRS}")
Expand Down Expand Up @@ -316,8 +310,10 @@ endif(USE_NVTX)
###################################################################################################
# - link libraries --------------------------------------------------------------------------------

target_link_libraries(cuspatial cudf cudart cuda cusparse nvrtc GDAL::GDAL)

target_link_libraries(cuspatial cudart cusparse GDAL::GDAL)
# Because libcudf.so doesn't contain any symbols, the linker will determine that it's okay to prune
# it before copying `DT_NEEDED` entries from it
target_link_libraries(cuspatial "-Wl,--no-as-needed" cudf "-Wl,--as-needed")

###################################################################################################
# - install targets -------------------------------------------------------------------------------
Expand Down
9 changes: 6 additions & 3 deletions cpp/benchmarks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,12 @@ function(ConfigureBench CMAKE_BENCH_NAME CMAKE_BENCH_SRC)
${CMAKE_BENCH_SRC}
"${CMAKE_CURRENT_SOURCE_DIR}/synchronization/synchronization.cpp")
set_target_properties(${CMAKE_BENCH_NAME} PROPERTIES POSITION_INDEPENDENT_CODE ON)
target_link_libraries(${CMAKE_BENCH_NAME} benchmark benchmark_main pthread cuspatial cudf
cudftestutil cudart cuda "${ARROW_LIB}" ${ZLIB_LIBRARIES}
nvrtc GDAL::GDAL)
# By default the linker doesn't transitively add `DT_NEEDED` entries in executables like it does
# for shared libraries, so to work around current libcudf build behavior we're adding the linker
# flag
target_link_libraries(${CMAKE_BENCH_NAME} "-Wl,--copy-dt-needed-entries")
target_link_libraries(${CMAKE_BENCH_NAME} benchmark benchmark_main pthread cuspatial
cudftestutil)
set_target_properties(${CMAKE_BENCH_NAME} PROPERTIES
RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/gbenchmarks")
endfunction(ConfigureBench)
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/spatial/polygon_bounding_box.cu
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ std::unique_ptr<cudf::table> compute_polygon_bounding_boxes(cudf::column_view co
point_ids.begin(),
thrust::maximum<int32_t>());

return std::move(point_ids);
return point_ids;
}();

auto type = cudf::data_type{cudf::type_to_id<T>()};
Expand Down
17 changes: 8 additions & 9 deletions cpp/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ function(ConfigureTest CMAKE_TEST_NAME CMAKE_TEST_SRC)
add_executable(${CMAKE_TEST_NAME}
${CMAKE_TEST_SRC})
set_target_properties(${CMAKE_TEST_NAME} PROPERTIES POSITION_INDEPENDENT_CODE ON)
target_link_libraries(${CMAKE_TEST_NAME} gmock gtest gmock_main gtest_main pthread cuspatial cudf
cudftestutil cudart cuda "${ARROW_LIB}" ${ZLIB_LIBRARIES}
nvrtc GDAL::GDAL)
# By default the linker doesn't transitively add `DT_NEEDED` entries in executables like it does
# for shared libraries, so to work around current libcudf build behavior we're adding the linker
# flag
target_link_libraries(${CMAKE_TEST_NAME} "-Wl,--copy-dt-needed-entries")
target_link_libraries(${CMAKE_TEST_NAME} gmock gtest gmock_main gtest_main pthread cuspatial
cudftestutil)
if(USE_NVTX)
target_link_libraries(${CMAKE_TEST_NAME} ${NVTX_LIBRARY})
endif(USE_NVTX)
Expand All @@ -67,7 +70,6 @@ include_directories("${CMAKE_BINARY_DIR}/include"
"${CMAKE_SOURCE_DIR}/include"
"${CMAKE_SOURCE_DIR}"
"${CMAKE_SOURCE_DIR}/src"
"${GDAL_INCLUDE_DIRS}"
"${GTEST_INCLUDE_DIR}"
"${RMM_INCLUDE}"
"${CUDF_INCLUDE}")
Expand All @@ -76,12 +78,9 @@ include_directories("${CMAKE_BINARY_DIR}/include"
# - library paths ---------------------------------------------------------------------------------

link_directories("${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}" # CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES is an undocumented/unsupported variable containing the link directories for nvcc
"${CMAKE_BINARY_DIR}/lib"
"${GDAL_LIBRARIES}"
"${CMAKE_BINARY_DIR}"
"${CONDA_LINK_DIRS}"
"${GTEST_LIBRARY_DIR}"
"${CUDF_LIBRARY}"
"${CUSPATIAL_LIBRARY}")
"${GTEST_LIBRARY_DIR}")

set(CARTESIAN_PRODUCT_GROUP_INDEX_ITERATOR_TEST_SRC
"${CMAKE_CURRENT_SOURCE_DIR}/spatial/cartesian_product_group_index_iterator_test.cpp")
Expand Down
4 changes: 2 additions & 2 deletions python/cuspatial/cuspatial/core/gis.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def directed_hausdorff_distance(xs, ys, points_per_space):
)
result = result.data_array_view
result = result.reshape(num_spaces, num_spaces)
return DataFrame.from_gpu_matrix(result)
return DataFrame(result)


def haversine_distance(p1_lon, p1_lat, p2_lon, p2_lat):
Expand Down Expand Up @@ -249,7 +249,7 @@ def point_in_polygon(
result = gis_utils.pip_bitmap_column_to_binary_array(
polygon_bitmap_column=result, width=len(poly_offsets)
)
result = DataFrame.from_gpu_matrix(result)
result = DataFrame(result)
result = result._apply_support_method("astype", dtype="bool")
result.columns = [x for x in list(reversed(poly_offsets.index))]
result = result[list(reversed(result.columns))]
Expand Down

0 comments on commit 6c2b52b

Please sign in to comment.