-
-
Notifications
You must be signed in to change notification settings - Fork 1
✨ Python bindings for CudaCogReader #58
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
Draft
weiji14
wants to merge
20
commits into
main
Choose a base branch
from
dlpack_to_cupy
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
682fae0 to
58cdee2
Compare
Setting up boilerplate code for using CudaCogReader in Python to read GeoTIFFs into a DLPack and then transfer to CuPy! Temporarily using unsafe impl Send and Sync to workaround raw pointer in CudaCogReader not being able to be shared between threads safely. Still need to properly handle some kwargs in __dlpack__ for cupy.from_dlpack(). A cupy array is returned with the correct shape, but numbers are wrong, possibly because pointer isn't managed properly.
58cdee2 to
6499ec9
Compare
Using bundled bindings to get compilation to work on CI. Tests with cuda should still be skipped since GPU CI is not available. Also bumped from cudarc 0.17.3 to 0.17.4.
Since nvTIFF isn't on osx, and can't be bothered with Windows yet. Do `maturin build --features cuda` on Linux CI tests (Python) only.
CudaCogReader might not be available on some platforms, so hide it behind some gates.
3 tasks
For some reason, calling CudaCogReader twice makes things work, i.e. the returned cupy.ndarray has the correct numbers. Thinking it might be some CUDA stream issue (https://docs.cupy.dev/en/v13.6.0/user_guide/basic.html#current-stream), but cupy should already be using the default null stream by default. Putting some print() and dbg!() statements here and there. Bumped cupy-cuda12x to cupy-cuda13x.
76e29f1 to
aed13e5
Compare
Install nvTIFF binaries from nvidia repos following instructions on https://developer.nvidia.com/nvtiff-0-5-0-download-archive?target_os=Linux&target_arch=x86_64&Distribution=Ubuntu&target_version=24.04&target_type=deb_network. Could have tried to get it from PyPI following https://docs.nvidia.com/cuda/nvtiff/installation.html#pypi, but then would need to figure out the lib paths and stuff.
aed13e5 to
cb05982
Compare
5e0408c to
0056bbf
Compare
Fix `Unable to find libclang: "couldn't find any valid shared libraries matching: ['libclang.so', 'libclang-*.so', 'libclang.so.*', 'libclang-*.so.*'], set the `LIBCLANG_PATH` environment variable to a path where one of these files can be found (invalid: [])"`. Need to install this inside the manylinux_2_28 docker container.
0056bbf to
a409689
Compare
Depending on which manylinux_2_28 docker image is pulled for each target arch, the underlying distribution could either be AlmaLinux or Ubuntu based, so need to handle either way of installing nvTIFF and clang-dev.
Fix nvtiff-sys compilation errors by installing missing CUDA runtime dependencies (cuda-crt and cuda-cudart-devel) and patching the nvtiff.h file following https://docs.rs/nvtiff-sys/0.1.2/nvtiff_sys/#instructions
Default `pyo3` flag set in pyproject.toml is overidden when passing `--features` flag to maturin build, so need to set `cuda,pyo3` instead. Also copy code from e75d171 to free-threaded build section.
Too tricky to get nvTIFF working on armv7, s390x and ppc64le due to some linker error like `/usr/armv7-unknown-linux-gnueabihf/lib/gcc/armv7-unknown-linux-gnueabihf/7.5.0/../../../../armv7-unknown-linux-gnueabihf/bin/ld: cannot find -lnvtiff`, so disabling them on those platforms.
715ca46 to
1c5965f
Compare
f0d0e16 to
1c6108a
Compare
Need to include 'cuda' feature flag to maturin on ReadtheDocs, and get libnvtiff-dev from conda-forge. Added a warning to the docstring indicating that CudaCogReader is experimental, and only available on linux-x86_64 and linux-aarch64 builds.
4c8a4b2 to
ae17b83
Compare
Point to where the header files are located. nvtiff.h is in $CONDA_PREFIX/include. cuda_runtime.h and crt/host_config.h are in $CONDA_PREFIX/targets/x86_64-linux/include.
ae17b83 to
6cc193b
Compare
Not sure if raw pointer in CudaCogReader is thread-safe enough to do `unsafe impl Send/Sync`, so using unsendable instead for now. Xref https://pyo3.rs/v0.27.1/migration.html#pyclass-structs-must-now-be-send-or-unsendable
Check that stream and max_version arguments are valid. Currently only supporting stream=1 or None, and DLPack version 1.x (dlpark is using DLPack 1.1). Have added some docstrings for these parameters. Not implementing copy kwarg yet though.
Bump nvtiff-sys from 0.1.2 to 0.1.3 to get Error trait on NvtiffStatusError, and then we can cast to string and pass error message to PyValueError.
Brute force symlinking to get nvtiff-sys to compile with conda-forge's libnvtiff that is under $CONDA_PREFIX/include/ instead of $CONDA_PREFIX/targets/x86_64-linux/include/ where most other header files are. One key part is to use RUSTFLAGS instead of LD_LIBRARY_PATH to actually get rustc to search the correct lib/ folder for the .so files.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow Python users to use the nvTIFF backend, reading a GeoTIFF into a DLPack that can be consumed by CuPy, Pytorch (2.9+), or any other Python library that implements the Python Specification for DLPack.
Preview at https://cog3pio--58.org.readthedocs.build/en/58/api/#dlpack
Usage:
TODO:
__dlpack__kwargs forcupy.from_dlpack()- [ ] dl_device(TODO next time)- [ ] copy(TODO next time)External things to do to improve implementation here:
nvtiff_sys::result::NvTiffErrorcupyas dependency so thatcupy-cuda13xcan be used insteadImportError: libnvtiff.so.0: cannot open shared object file: No such file or directorywhen runningimport cog3piowithout nvTIFF installed (e.g. if they don't have a CUDA GPU). Might need to look into https://wheelnext.dev/proposals/pepxxx_wheel_variant_support/. Temporary workaround might be to have the nvTIFF support added to the free-threaded builds only?References:
Follow-up to #57, part of #26