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.

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

Jump to bottom

Nvcomp revert followup #2497

Merged
merged 4 commits into from
Oct 11, 2024
Merged

Nvcomp revert followup #2497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8122410
Update the patch to apply on latest version of CUDF
revans2 Oct 11, 2024
ac144ee
Review rework
revans2 Oct 11, 2024
4675015
Got the right patch this time
revans2 Oct 11, 2024
cdd4db0
Better patch
revans2 Oct 11, 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
2 changes: 1 addition & 1 deletion ci/submodule-sync.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ echo "Test against ${cudf_sha}..."
MVN="mvn -Dmaven.wagon.http.retryHandler.count=3 -B"
set +e
# Don't do a full build. Just try to update/build CUDF with no patches on top of it.
${MVN} validate ${MVN_MIRROR} \
${MVN} antrun:run@build-libcudf ${MVN_MIRROR} \
-DCPP_PARALLEL_LEVEL=${PARALLEL_LEVEL} \
-Dlibcudf.build.configure=true \
-Dlibcudf.dependency.mode=latest \
Expand Down
82 changes: 57 additions & 25 deletions patches/revert_nvcomp4.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ index 5e9f7f8a0c..0e4745bda2 100755
${package_dir}/dist/*

diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml
index 5a05dfd053..e7363645d6 100644
index bd5e6c3d56..74ca3fda1a 100644
--- a/conda/environments/all_cuda-118_arch-x86_64.yaml
+++ b/conda/environments/all_cuda-118_arch-x86_64.yaml
@@ -58,7 +58,7 @@ dependencies:
Expand All @@ -38,18 +38,27 @@ index 5a05dfd053..e7363645d6 100644
- openpyxl
- packaging
diff --git a/conda/environments/all_cuda-125_arch-x86_64.yaml b/conda/environments/all_cuda-125_arch-x86_64.yaml
index 8490296233..3559a1a341 100644
index 565a3ebfa3..22619acf4a 100644
--- a/conda/environments/all_cuda-125_arch-x86_64.yaml
+++ b/conda/environments/all_cuda-125_arch-x86_64.yaml
@@ -56,7 +56,7 @@ dependencies:
- numba>=0.57
- numba-cuda>=0.0.13
- numpy>=1.23,<3.0a0
- numpydoc
-- nvcomp==4.0.1
+- nvcomp==3.0.6
- nvtx>=0.2.1
- openpyxl
- packaging
@@ -67,7 +67,7 @@ dependencies:
- pre-commit
- pyarrow>=14.0.0,<18.0.0a0
- pydata-sphinx-theme!=0.14.2
-- pynvjitlink>=0.0.0a0
+- pynvjitlink
- pytest-benchmark
- pytest-cases>=3.8.2
- pytest-cov
diff --git a/conda/recipes/libcudf/conda_build_config.yaml b/conda/recipes/libcudf/conda_build_config.yaml
index dc75eb4b25..67d501d746 100644
--- a/conda/recipes/libcudf/conda_build_config.yaml
Expand Down Expand Up @@ -697,7 +706,7 @@ index 60a64fb0ee..40cfbe763b 100644

/**
diff --git a/cpp/src/io/parquet/reader_impl_chunking.cu b/cpp/src/io/parquet/reader_impl_chunking.cu
index c588fedb85..bab70c126b 100644
index 27312a4da8..779d40281b 100644
--- a/cpp/src/io/parquet/reader_impl_chunking.cu
+++ b/cpp/src/io/parquet/reader_impl_chunking.cu
@@ -865,18 +865,8 @@ std::vector<row_range> compute_page_splits_by_row(device_span<cumulative_page_in
Expand Down Expand Up @@ -735,10 +744,10 @@ index 396d44c076..e2f09f872d 100644

size_t max_compression_output_size(Compression codec, uint32_t compression_blocksize)
diff --git a/cpp/tests/io/comp/decomp_test.cpp b/cpp/tests/io/comp/decomp_test.cpp
index 840cf263ed..38c1a57eca 100644
index 54262dc3b4..3b8549c9f4 100644
--- a/cpp/tests/io/comp/decomp_test.cpp
+++ b/cpp/tests/io/comp/decomp_test.cpp
@@ -176,19 +176,23 @@ TEST_F(NvcompConfigTest, Compression)
@@ -184,19 +184,23 @@ TEST_F(NvcompConfigTest, Compression)
using cudf::io::nvcomp::compression_type;
auto const& comp_disabled = cudf::io::nvcomp::is_compression_disabled;

Expand Down Expand Up @@ -770,7 +779,7 @@ index 840cf263ed..38c1a57eca 100644
}

TEST_F(NvcompConfigTest, Decompression)
@@ -196,19 +200,27 @@ TEST_F(NvcompConfigTest, Decompression)
@@ -204,19 +208,27 @@ TEST_F(NvcompConfigTest, Decompression)
using cudf::io::nvcomp::compression_type;
auto const& decomp_disabled = cudf::io::nvcomp::is_decompression_disabled;

Expand Down Expand Up @@ -808,18 +817,53 @@ index 840cf263ed..38c1a57eca 100644

CUDF_TEST_PROGRAM_MAIN()
diff --git a/dependencies.yaml b/dependencies.yaml
index 6909eb7168..2fa58a31b1 100644
index ca17917c90..1ec1f5f6da 100644
--- a/dependencies.yaml
+++ b/dependencies.yaml
@@ -368,7 +368,7 @@ dependencies:
- flatbuffers==24.3.25
- librdkafka>=2.5.0,<2.6.0a0
@@ -381,21 +381,21 @@ dependencies:
- output_types: conda
packages:
# Align nvcomp version with rapids-cmake
- - nvcomp==4.0.1
+ - nvcomp==3.0.6
- spdlog>=1.14.1,<1.15
specific:
- output_types: [requirements, pyproject]
matrices:
- matrix:
cuda: "12.*"
packages:
- - nvidia-nvcomp-cu12==4.0.1
+ - nvidia-nvcomp-cu12==3.0.6
- matrix:
cuda: "11.*"
packages:
- - nvidia-nvcomp-cu11==4.0.1
+ - nvidia-nvcomp-cu11==3.0.6
- matrix:
packages:
- - nvidia-nvcomp==4.0.1
+ - nvidia-nvcomp==3.0.6
rapids_build_skbuild:
common:
- output_types: [conda, requirements, pyproject]
@@ -665,7 +665,7 @@ dependencies:
matrices:
- matrix: {cuda: "12.*"}
packages:
- - &pynvjitlink_unsuffixed pynvjitlink>=0.0.0a0
+ - &pynvjitlink_unsuffixed pynvjitlink
- matrix: {cuda: "11.*"}
packages:
- &cubinlinker_unsuffixed cubinlinker
@@ -676,7 +676,7 @@ dependencies:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- - pynvjitlink-cu12>=0.0.0a0
+ - pynvjitlink-cu12
- matrix:
cuda: "12.*"
cuda_suffixed: "false"
diff --git a/docs/cudf/source/user_guide/io/io.md b/docs/cudf/source/user_guide/io/io.md
index 97b961b455..adcdaa51e7 100644
--- a/docs/cudf/source/user_guide/io/io.md
Expand All @@ -843,7 +887,7 @@ index 97b961b455..adcdaa51e7 100644
-
```
diff --git a/java/pom.xml b/java/pom.xml
index e4f1cdf64e..9694e741f1 100644
index 450cfbdbc8..55cb055398 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -1,6 +1,6 @@
Expand Down Expand Up @@ -893,15 +937,3 @@ index 32045f3c50..c18a90140b 100644
COMMENT "Copying nvcomp libraries to ${PROJECT_BINARY_DIR}"
)
endif()
diff --git a/python/libcudf/CMakeLists.txt b/python/libcudf/CMakeLists.txt
index 0a8f5c4807..96eb6c3bb3 100644
--- a/python/libcudf/CMakeLists.txt
+++ b/python/libcudf/CMakeLists.txt
@@ -48,5 +48,6 @@ add_subdirectory(../../cpp cudf-cpp)
# Ensure other libraries needed by libcudf.so get installed alongside it.
include(cmake/Modules/WheelHelpers.cmake)
install_aliased_imported_targets(
- TARGETS cudf nvcomp::nvcomp DESTINATION ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}
+ TARGETS cudf nvcomp::nvcomp nvcomp::nvcomp_gdeflate nvcomp::nvcomp_bitcomp DESTINATION
+ ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}
)
8 changes: 6 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@
<id>build-libcudf</id>
<phase>validate</phase>
<configuration>
<target xmlns:if="ant:if">
<target xmlns:if="ant:if" xmlns:unless="ant:unless">
<condition property="needConfigure">
<or>
<istrue value="${libcudf.build.configure}"/>
Expand Down Expand Up @@ -466,7 +466,8 @@
</exec>
<exec dir="${libcudf.build.path}"
failonerror="true"
executable="cmake">
executable="cmake"
unless:true="${submodule.patch.skip}">
<arg value="--build"/>
<arg value="${libcudf.build.path}"/>
<arg value="--target"/>
Expand All @@ -483,6 +484,7 @@
<id>build-libcudfjni</id>
<phase>validate</phase>
<configuration>
<skip>${submodule.patch.skip}</skip>
<target>
<mkdir dir="${libcudfjni.build.path}"/>
<exec dir="${libcudfjni.build.path}"
Expand Down Expand Up @@ -518,6 +520,7 @@
<id>build-sparkrapidsjni</id>
<phase>validate</phase>
<configuration>
<skip>${submodule.patch.skip}</skip>
Copy link
Member

Choose a reason for hiding this comment

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

Why are we skipping building the C++ code if we're skipping patching? This prevents building with subsequent changes on a followup build that refuses to proceed if it's already been patched.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gerashegalov this was your suggestion. I assume that it was to avoid building CUDF in an unpatched state.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I tried to get rid of unnecessary work in the validate phase of the original PR assuming that we don't want to build anything unless the repo is patched.

<target>
<mkdir dir="${native.build.path}"/>
<exec dir="${native.build.path}"
Expand Down Expand Up @@ -556,6 +559,7 @@
<id>build-info</id>
<phase>generate-resources</phase>
<configuration>
<skip>${submodule.patch.skip}</skip>
<target>
<mkdir dir="${project.build.directory}/extra-resources"/>
<exec executable="bash"
Expand Down
8 changes: 4 additions & 4 deletions thirdparty/cudf-pins/versions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,15 @@
"git_url" : "https://github.com/NVIDIA/nvcomp.git",
"proprietary_binary" :
{
"aarch64-linux" : "https://developer.download.nvidia.com/compute/nvcomp/${version}/local_installers/nvcomp-linux-sbsa-${version}-cuda${cuda-toolkit-version-mapping}.tar.gz",
"x86_64-linux" : "https://developer.download.nvidia.com/compute/nvcomp/${version}/local_installers/nvcomp-linux-x86_64-${version}-cuda${cuda-toolkit-version-mapping}.tar.gz"
"aarch64-linux" : "https://developer.download.nvidia.com/compute/nvcomp/${version}/local_installers/nvcomp_${version}_SBSA_${cuda-toolkit-version-mapping}.tgz",
"x86_64-linux" : "https://developer.download.nvidia.com/compute/nvcomp/${version}/local_installers/nvcomp_${version}_x86_64_${cuda-toolkit-version-mapping}.tgz"
},
"proprietary_binary_cuda_version_mapping" :
{
"11" : "11.x",
"12" : "12.x"
},
"version" : "4.0.1"
"version" : "3.0.6"
},
"nvtx3" :
{
Expand All @@ -162,4 +162,4 @@
"version" : "1.14.1"
}
}
}
}
Loading