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

Initial Python bindings for cuProj #1217

Merged
merged 219 commits into from
Aug 2, 2023
Merged

Initial Python bindings for cuProj #1217

merged 219 commits into from
Aug 2, 2023

Conversation

harrism
Copy link
Member

@harrism harrism commented Jul 13, 2023 •
edited
Loading

Description

Closes #1213

This benchmark is run on a DGX H100. See the cuproj_benchmark.ipynb included with this PR. PyProj results use a single Xeon core from this machine, and cuProj results use a single H100 GPU. cuProj speedup for double precision, data on device (see notebook):

cuProj Speedup for 1,000,000,000 points: 4103.17x

image

Note this PR also includes a number of C++ changes that were necessary to enable Python/Cython bindings, and/or to enable compatibility with PyProj (e.g. refactored EPSG string parsing class).

TODO:

  • Support arrays
  • More comprehensive tests, including a grid of coordinates
  • Test inverse transforms
  • Support __cuda_array_interface__
  • Support 32-bit floats
  • Update CI scripts to build cuProj Python bindings and run pytests
  • Documentation -- in follow-up PR cuProj Python and C++ Documentation #1237
  • Support interleaved coordinates
  • Support axis order the way PyProj does (e.g. not always lat, lon) (actually, this does now work the way PyProj does for the transformation we support, which requires (lat, lon) ordering for WGS84, and outputs (Easting, Northing) order.
  • But we could add support for always_xy parameter that PyProj has.
  • Support integer epsg code arguments in Transformer.from_crs
  • Support mixed integer and string epsg code arguments in Transformer.from_crs
  • Support tuples of ("EPSG", code) in Transformer.from_crs
  • Fix projection factory to not return a raw pointer
  • cuprojshim clang-format
  • Benchmark notebook

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

harrism and others added 30 commits May 9, 2023 04:56
@harrism
Add cmake infrastructure and placeholder files for projection library
cfd69ad
@harrism
Merge branch 'branch-23.06' into feature/projection/cmake
dcb23ef
@harrism
Rename to cuproj
7b2e9b9
@harrism
Merge branch 'branch-23.06' into feature/projection/cmake
f469cf0
@harrism
Delete cugeo dir
68acf29
@harrism
show more digits in vec_2d_matcher
c16e735
@harrism
Add copy of PROJ license
818a3e5
@harrism
Initial tmerc implementation and test
c1550cd
@harrism
Merge branch 'branch-23.08' into feature/projection/cmake
c2784a9
@harrism
Remove _ctk_static_suffix
d8d851d
@harrism
if TARGET conda_env update
dade592
@harrism
Merge branch 'branch-23.08' into feature/projection/cmake
10b0590
@harrism
Update version to 23.08 and PUBLIC -> PRIVATE
88a3760
@harrism
Merge branch 'feature/projection/cmake' into feature/projection/wgs-utm
c36c7eb
@harrism
Move PROJ license into the file that uses its code.
2be7746
@harrism
Remove placeholder CPU code
87172f0
@harrism
Rename test.
8cdc8dc
@harrism
Move cuproj into cpp and build it when cuspatial builds.
48ead2e
@harrism
re-add cuml
b889452
@harrism
Add sqlite dependency
2073b1f
@harrism
Merge branch 'branch-23.08' into feature/projection/cmake
d366ad5
@harrism
Add proj and sqlite dependencies to conda meta.yaml
d59cdc4
@harrism
Merge branch 'branch-23.08' into feature/projection/cmake
3eba34e
@harrism
Fix comment
cd68e4e
@harrism
rename to get_cudf.cmake
5243a5d
@harrism
Remove GNUInstallDirs
e27f7bb
@harrism
Copyright
a0ad375
@harrism
cuspatial-exports --> cuproj-exports
e2a28ea
@harrism
PROJ namespace
5d395c4
@harrism
Remove FIND_PACKAGE_ARGUMENTS
517e65b
@harrism harrism requested review from isVoid and bdice August 2, 2023 00:37
vyasr
vyasr approved these changes Aug 2, 2023
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There's a few small issues, but mostly good! Approving assuming my and @bdice's comments are addressed.

ci/test_wheel_cuproj.sh Show resolved Hide resolved
conda/environments/all_cuda-118_arch-x86_64.yaml Outdated Show resolved Hide resolved
dependencies.yaml Outdated Show resolved Hide resolved
conda/recipes/cuproj/meta.yaml Show resolved Hide resolved
conda/recipes/cuproj/build.sh Outdated Show resolved Hide resolved
@harrism harrism mentioned this pull request Aug 2, 2023
Closed
isVoid
isVoid approved these changes Aug 2, 2023
View reviewed changes
ci/release/apply_wheel_modifications.sh Outdated Show resolved Hide resolved
conda/recipes/cuproj/meta.yaml Show resolved Hide resolved
cpp/cuproj/tests/test.cu Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove the cuproj test?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was a placeholder before I wrote wgs_to_utm_test.cu. It was an empty file, just to test cmake.

python/cuproj/cuproj/_lib/transform.pyx Outdated Show resolved Hide resolved
python/cuproj/pyproject.toml Outdated Show resolved Hide resolved
bdice
bdice approved these changes Aug 2, 2023
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! One minor comment, nonblocking. (And a reply to an existing thread.)

conda/recipes/cuproj/build.sh Outdated Show resolved Hide resolved
conda/recipes/cuproj/meta.yaml Show resolved Hide resolved
@harrism
Copy link
Member Author

harrism commented Aug 2, 2023

These are my highest approval ratings yet! Maybe I should run for president?

@harrism
Copy link
Member Author

harrism commented Aug 2, 2023

/merge

@raydouglass raydouglass mentioned this pull request Aug 2, 2023
Merged
@rapids-bot rapids-bot bot merged commit 6b4476b into rapidsai:branch-23.08 Aug 2, 2023
55 checks passed
@isVoid isVoid mentioned this pull request Aug 2, 2023
Closed
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Aug 3, 2023
@jarmak-nv
cuSpatial 23.08 README updates (#1232)
d55fb40 
closes #1205 

This PR updates the cuSpatial Readme per requirements in #1205 (as well as the update-version.sh).

Depends on #1237 and #1217

Authors:
  - Ben Jarmak (https://github.com/jarmak-nv)

Approvers:
  - Mark Harris (https://github.com/harrism)

URL: #1232
rapids-bot bot pushed a commit that referenced this pull request Aug 3, 2023
@harrism
cuProj Python and C++ Documentation (#1237)
011c5f7 
Adds doxygen configuration for libcuproj, and Python documentation.

Depends on #1217 
Fixes #1239

Authors:
  - Mark Harris (https://github.com/harrism)
  - Michael Wang (https://github.com/isVoid)
  - AJ Schmidt (https://github.com/ajschmidt8)

Approvers:
  - Ben Jarmak (https://github.com/jarmak-nv)
  - H. Thomson Comer (https://github.com/thomcom)
  - Paul Taylor (https://github.com/trxcllnt)
  - Michael Wang (https://github.com/isVoid)
  - Ray Douglass (https://github.com/raydouglass)

URL: #1237
@raydouglass raydouglass mentioned this pull request Aug 3, 2023
Merged
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Aug 3, 2023
@raydouglass
Update nightly wheel tests for cuproj (#1250)
a09cc0d 
Follow up to #1217 to enable wheels testing for `cuproj` for nightly tests.

Should fix issue seen [here](https://github.com/rapidsai/cuspatial/actions/runs/5748659541/job/15581971500)

Authors:
  - Ray Douglass (https://github.com/raydouglass)

Approvers:
  - AJ Schmidt (https://github.com/ajschmidt8)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #1250
rapids-bot bot pushed a commit to rapidsai/integration that referenced this pull request Aug 3, 2023
@jarmak-nv
Add cuProj to the RAPIDS meta package (#674)
72f0ca7 
Small PR to add `cuProj` to the RAPIDS metapackages.

Depends on rapidsai/cuspatial#1217

Authors:
  - Ben Jarmak (https://github.com/jarmak-nv)

Approvers:
  - Ray Douglass (https://github.com/raydouglass)

URL: #674
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vyasr vyasr vyasr approved these changes

@thomcom thomcom thomcom approved these changes

@bdice bdice bdice approved these changes

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

@isVoid isVoid isVoid approved these changes

@jarmak-nv jarmak-nv jarmak-nv approved these changes

@trxcllnt trxcllnt Awaiting requested review from trxcllnt trxcllnt is a code owner automatically assigned from rapidsai/cuspatial-python-codeowners

@robertmaynard robertmaynard Awaiting requested review from robertmaynard

Assignees

@harrism harrism

Labels
4 - Needs Reviewer Waiting for reviewer to review or respond ci cmake Related to CMake code or build configuration conda Related to conda and conda configuration feature request New feature or request libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change Python Related to Python code
Projects
cuSpatial
Status: Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Python bindings for cuProj transformation (e.g. UTM <-> WGS)
7 participants
@harrism @thomcom @vyasr @bdice @ajschmidt8 @isVoid @jarmak-nv