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

Update nvcomp to 3.0.4. #313

Closed
wants to merge 3 commits into from

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Nov 2, 2023

Description

Update the nvCOMP version used for compression/decompression to 3.0.4.

See also:

@jakirkham jakirkham added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Nov 2, 2023
@jakirkham
Copy link
Member

Actually am now wondering, should we consider moving to nvCOMP 3 breaking?

@bdice bdice added breaking Introduces a breaking change and removed non-breaking Introduces a non-breaking change labels Nov 2, 2023
@bdice
Copy link
Contributor Author

bdice commented Nov 2, 2023

We can mark it as breaking, to alert users in case it matters.

@bdice
Copy link
Contributor Author

bdice commented Nov 2, 2023

We'll have to disable devcontainers CI for this PR to pass, because it has a circular dependency with cudf. Done in 8654a3c.

@bdice
Copy link
Contributor Author

bdice commented Nov 2, 2023

@madsbk Can you take a peek at this PR? I think there might be some nvcomp APIs that need to be updated for version 3.0.4, but I'm not entirely sure how kvikio works here.

  /opt/conda/conda-bld/work/python/_skbuild/linux-x86_64-3.9/cmake-build/kvikio/_lib/libnvcomp.cxx: In function 'PyObject* __pyx_pf_6kvikio_4_lib_9libnvcomp_14_nvcompManager_10set_scratch_buffer(__pyx_obj_6kvikio_4_lib_9libnvcomp__nvcompManager*, __pyx_obj_6kvikio_4_lib_3arr_Array*)':
  /opt/conda/conda-bld/work/python/_skbuild/linux-x86_64-3.9/cmake-build/kvikio/_lib/libnvcomp.cxx:19255:26: error: 'struct nvcomp::nvcompManagerBase' has no member named 'set_scratch_buffer'
  19255 |     __pyx_v_self->_impl->set_scratch_buffer(((uint8_t *)__pyx_v_new_scratch_buffer->ptr));
        |                          ^~~~~~~~~~~~~~~~~~

@bdice bdice marked this pull request as ready for review November 3, 2023 00:05
@bdice bdice requested review from a team as code owners November 3, 2023 00:05
@madsbk
Copy link
Member

madsbk commented Nov 3, 2023

I am not able to create the conda environment with nvcomp==3.0.4.

$ mamba env create --name kvikio-nvcomp-3.0.4 --file conda/environments/all_cuda-120_arch-x86_64.yaml
conda-forge/linux-64     Using cache
conda-forge/noarch       Using cache
nvidia/linux-64          Using cache
nvidia/noarch            Using cache
pytorch/linux-64         Using cache
pytorch/noarch           Using cache
rapidsai-nightly/linux-6 [====================] (00m:00s) No change
rapidsai/noarch          [====================] (00m:00s) No change
rapidsai/linux-64        [====================] (00m:00s) No change
rapidsai-nightly/noarch  [====================] (00m:00s) No change


Looking for: ['c-compiler', "cmake[version='>=3.26.4']", 'cuda-nvcc', "cuda-python[version='>=12.0,<13.0a0']", 'cuda-version=12.0', 'cudf=23.12', "cupy[version='>=12.0.0']", 'cxx-compiler', "cython[version='>=3.0.0']", "dask[version='>=2022.05.2']", "distributed[version='>=2022.05.2']", 'doxygen=1.9.1', 'gcc_linux-64=11', 'libcufile', 'libcufile-dev', 'ninja', "numcodecs[version='<0.12.0']", "numpy[version='>=1.21']", 'numpydoc', 'nvcomp==3.0.4', 'packaging', 'pre-commit', 'pytest', 'pytest-cov', 'python==3.10', "scikit-build[version='>=0.13.1']", 'sphinx', 'sphinx-click', 'sphinx_rtd_theme', 'sysroot_linux-64=2.17', 'zarr']


Encountered problems while solving:
  - nothing provides _python_rc needed by python-3.12.0rc3-rc3_hab00c5b_1_cpython

Any thoughts?

@bdice
Copy link
Contributor Author

bdice commented Nov 3, 2023

@madsbk This is failing for the same reason I had to disable devcontainers -- current builds of libcudf are pinned to nvcomp==2.6.1. There is a circular dependency between libkvikio (which is used to build libcudf) and cudf (which is used to test kvikio). Try removing cudf=23.12 from the environment.

@vuule
Copy link
Contributor

vuule commented Nov 3, 2023

I tried my hand at implementing the changes required due to the API change, but I can't get the functors in Cython quite right
#314
I hope it's at least useful as a starting point.

@bdice
Copy link
Contributor Author

bdice commented Nov 9, 2023

Closing this PR in favor of #314.

@bdice bdice closed this Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Introduces a breaking change improvement Improves an existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants