Fix cloning, compilation and linking errors for the non-docker version #9

gprabowski · 2023-05-01T20:13:20Z

Currently the repository is impossible to clone or build in both docker and non-docker version.

Although not full, this is a list of changes that makes it work for the local non-docker version.

There's much more to be done, to make it universal, but this is the minimal set.

More changes will be added as the semester progresses.

steps to build, assuming conda rapids-23.04 environment has been installed (if not install from here)

git clone --recurse-submodules [...]
cd meta-json-parser
mkdir build
cd build
conda activate rapids-23.04
cmake ../ -GNinja
ninja

gprabowski · 2023-05-01T20:13:55Z

.gitmodules

-	url = https://github.com/google/googletest.git
+[submodule "third_parties/fmt"]
+	path = third_parties/fmt
+	url = https://github.com/fmtlib/fmt.git
 [submodule "third_parties/mp11"]


otherwise impossible to clone

gprabowski · 2023-05-01T20:14:22Z

.gitmodules

-[submodule "third_parties/googletest"]
-	path = third_parties/googletest
-	url = https://github.com/google/googletest.git
+[submodule "third_parties/fmt"]


otherwise impossible to link

gprabowski · 2023-05-01T20:15:07Z

CMakeLists.txt

@@ -3,29 +3,22 @@ set(CMAKE_VERBOSE_MAKEFILE ON)

 project(meta-json-parser VERSION 0.1 LANGUAGES CXX CUDA)

-find_package(CUDAToolkit 10.0 REQUIRED)
+# As of now not using LIBCUDF throws
+set(USE_LIBCUDF 1)


if not set to 1, the project throws compilation errors (ifdefs are leaky)

gprabowski · 2023-05-01T20:15:45Z

CMakeLists.txt

-IF(NOT LOCAL_LIB)
-find_package(Boost 1.75)  # version 1.75 is required to use boost::mp11::mp_pairwise_fold_q
-ENDIF()
+find_package(CUDAToolkit EXACT 11.8 REQUIRED)


currently the newest is 12 (so the previous would find 12, as it only set minimum), but RAPIDS are currently limited to 11.X

gprabowski · 2023-05-01T20:15:59Z

CMakeLists.txt

 include(GoogleTest)

+#Add FMT library (as CUDA11.8 doesn't support gcc12)
+add_subdirectory(third_parties/fmt)


gprabowski · 2023-05-01T20:16:26Z

CMakeLists.txt

@@ -113,6 +106,7 @@ target_link_libraries(meta-cudf-parser-1 PUBLIC lib-meta-json-parser)

 target_include_directories(meta-cudf-parser-1 PUBLIC
    "${PROJECT_SOURCE_DIR}/meta_cudf/opt1"
+    "$ENV{CONDA_PREFIX}/include"


universal instead of hardfixed

gprabowski · 2023-05-01T20:16:37Z

CMakeLists.txt

@@ -182,7 +176,7 @@ add_subdirectory(third_parties/CLI11)
 #add_executable(meta-json-parser-benchmark benchmark/checkpoint_results.cpp benchmark/checkpoint_results.h)
 add_executable(meta-json-parser-benchmark)

-target_link_libraries(meta-json-parser-benchmark PRIVATE lib-meta-json-parser CLI11)
+target_link_libraries(meta-json-parser-benchmark PRIVATE lib-meta-json-parser fmt::fmt CLI11)


otherwise throws linking errors

During which part of build (I wonder which part requires fmt library)?

gprabowski · 2023-05-01T20:16:45Z

CMakeLists.txt

@@ -192,33 +186,34 @@ if (USE_LIBCUDF)
    #       instead of hardcoding paths to includes and to libraries, like below
    target_include_directories(meta-cudf-parser-1 PUBLIC
                               /usr/local/include
-                               /usr/local/include/libcudf/libcudacxx
+                               $ENV{CONDA_PREFIX}/include


universal instead of hardfixed

gprabowski · 2023-05-01T20:17:15Z

CMakeLists.txt

@@ -192,33 +186,34 @@ if (USE_LIBCUDF)
    #       instead of hardcoding paths to includes and to libraries, like below
    target_include_directories(meta-cudf-parser-1 PUBLIC
                               /usr/local/include
-                               /usr/local/include/libcudf/libcudacxx


couldn't find libcudacxx in any folder related to current libraries and deleting this doesn't change anything

I remember it was necessary for something, perhaps for running via docker image, but might be no longer needed.

gprabowski · 2023-05-01T20:17:29Z

CMakeLists.txt

    )
    target_include_directories(meta-cudf-parser-1 AFTER PUBLIC
-                               /opt/conda/envs/rapids/include
+        $ENV{CONDA_PREFIX}/include


universal instead of hardfixed

also /opt/conda/envs/rapids doesn't exist in a default conda installation of rapids

gprabowski · 2023-05-01T20:17:39Z

CMakeLists.txt

    )
    target_link_libraries(meta-cudf-parser-1 PRIVATE
                          cudf
                          -L/usr/local/cuda/lib64 -L/usr/local/lib
-                          -L/opt/conda/envs/rapids/lib
+                          -L$ENV{CONDA_PREFIX}/lib


universal instead of hardfixed

also /opt/conda/envs/rapids doesn't exist in a default conda installation of rapids

RAPIDS docker images used to have some of necessary includes and libs in /opt/conda/envs/rapids/ - while Dockerfile does not set CONDA_PREFIX environment variable. That's why it is here: for sudo docker build . to work.

Adding the $ENV{CONDA_PREFIX}/lib part is good, but I'd wait with removing /opt/conda/envs/rapids/ (see split commits).

gprabowski · 2023-05-01T20:18:18Z

CMakeLists.txt

    )
    target_compile_definitions(meta-cudf-parser-1 PRIVATE
                               HAVE_LIBCUDF=1
    )

    target_include_directories(meta-json-parser-benchmark PUBLIC
                               /usr/local/include
-                               /usr/local/include/libcudf/libcudacxx


gprabowski · 2023-05-01T20:18:29Z

CMakeLists.txt

    )
    target_compile_definitions(meta-cudf-parser-1 PRIVATE
                               HAVE_LIBCUDF=1
    )

    target_include_directories(meta-json-parser-benchmark PUBLIC
                               /usr/local/include
-                               /usr/local/include/libcudf/libcudacxx
+                               $ENV{CONDA_PREFIX}/include


gprabowski · 2023-05-01T20:18:38Z

CMakeLists.txt

    # NOTE: needs to be added as the last directory to be checked, because
    # it can contain outdated Boost library (which in Ubuntu 20.04 is Boost 1.72)
    target_include_directories(meta-json-parser-benchmark AFTER PUBLIC
-                               /opt/conda/envs/rapids/include
+        $ENV{CONDA_PREFIX}/include


gprabowski · 2023-05-01T20:19:06Z

CMakeLists.txt

    )
    target_link_libraries(meta-json-parser-benchmark PRIVATE
                          cudf
                          -L/usr/local/cuda/lib64 -L/usr/local/lib
-                          -L/opt/conda/envs/rapids/lib
+                          -L$ENV{CONDA_PREFIX}/lib


universal instead of hardfixed

as above with respect to existence of /envs/rapids

gprabowski · 2023-05-01T20:20:35Z

include/meta_json_parser/debug_helpers.cpp

@@ -192,7 +192,7 @@ void describe_table(cudf::io::table_with_metadata& table_with_metadata, bool dum
 	printf("table (with metadata) has %d columns\n", n_cols);
 	for (int col_idx = 0; col_idx < n_cols; col_idx++) {
 		printf("column(%d) name is \"%s\":\n", col_idx,
-		       table_with_metadata.metadata.column_names[col_idx].c_str());
+		       table_with_metadata.metadata.schema_info[col_idx].name.c_str());


cuDF in new (stable) RAPIDS released has changed its API and this is the new approach. schema info is a table of structs containing strings instead of table of strings

more info here

gprabowski · 2023-05-01T20:20:57Z

include/meta_json_parser/parser_output_host.cuh

@@ -1,7 +1,6 @@
 #pragma once 
 #include <cuda_runtime_api.h>
 #include <thrust/host_vector.h>
-#include <thrust/system/cuda/experimental/pinned_allocator.h>


not used and CUDA 12 toolchain incompatible

gprabowski · 2023-05-01T20:21:10Z

meta_cudf/parser.cu

@@ -257,7 +257,8 @@ cudf::io::table_with_metadata generate_example_metadata(const char* filename, in
        return "Column " + to_string(i++);
    });

-    cudf::io::table_metadata metadata{column_names};
+    cudf::io::table_metadata metadata;
+    std::for_each(begin(column_names), end(column_names), [&](auto& elem){metadata.schema_info.push_back({elem});});


as above (cuDF API change)

gprabowski · 2023-05-15T19:46:53Z

a thing I noticed in development:
CONDA overwrites CUDA_HOME, it should be set manually back to 11.8 instead of 12.1 to which for some reason conda defaults to (although rapids-23.04 explicitly needs 11.8)

jnareb · 2023-05-23T21:24:46Z

Thanks.

I am currently working on incorporating this pull request, splitting it into even smaller pieces.
You can see current work in progress in branch 'pg/cmake_improvements', see
master...pg/cmake_improvements

gprabowski added 3 commits May 1, 2023 20:48

Make all libraries local and fix include errors

1c9152f

Force highest CUDA compatible with RAPIDS

e0e0cae

Fix cuDF includes

47dace0

gprabowski commented May 1, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix cloning, compilation and linking errors for the non-docker version #9

Fix cloning, compilation and linking errors for the non-docker version #9

gprabowski commented May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

jnareb May 23, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

jnareb May 23, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

jnareb May 23, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski May 1, 2023

gprabowski commented May 15, 2023

jnareb commented May 23, 2023

Fix cloning, compilation and linking errors for the non-docker version #9

Are you sure you want to change the base?

Fix cloning, compilation and linking errors for the non-docker version #9

Conversation

gprabowski commented May 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gprabowski commented May 15, 2023

jnareb commented May 23, 2023