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

cuProj Python and C++ Documentation #1237

Merged
merged 248 commits into from
Aug 3, 2023

Conversation

harrism
Copy link
Member

@harrism harrism commented Jul 29, 2023

Description

Adds doxygen configuration for libcuproj, and Python documentation.

Depends on #1217
Fixes #1239

Checklist

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

jarmak-nv added a commit to jarmak-nv/cuspatial that referenced this pull request Aug 2, 2023
rapids-bot bot pushed a commit that referenced this pull request Aug 2, 2023
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](https://github.com/rapidsai/cuspatial/assets/783069/6d4e2a02-50fe-43db-a07d-fe71f91d1b89)

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:
 - [x] Support arrays
 - [x] More comprehensive tests, including a grid of coordinates
 - [x] Test inverse transforms
 - [x] Support `__cuda_array_interface__`
 - [x] Support 32-bit floats
 - [x] Update CI scripts to build cuProj Python bindings and run pytests
 - [x] ~Documentation~ -- in follow-up PR #1237 
 - [ ] Support interleaved coordinates
 - [x] 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.
 - [x] Support integer epsg code arguments in `Transformer.from_crs`
 - [x] Support mixed integer and string epsg code arguments in `Transformer.from_crs`
 - [x] Support tuples of ("EPSG", code) in `Transformer.from_crs`
 - [ ] Fix projection factory to not return a raw pointer
 - [x] cuprojshim clang-format
 - [x] Benchmark notebook

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

Approvers:
  - H. Thomson Comer (https://github.com/thomcom)
  - Ben Jarmak (https://github.com/jarmak-nv)
  - AJ Schmidt (https://github.com/ajschmidt8)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Michael Wang (https://github.com/isVoid)
  - Bradley Dice (https://github.com/bdice)

URL: #1217
Copy link
Contributor

@jarmak-nv jarmak-nv left a comment

Choose a reason for hiding this comment

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

Last couple of comments/updates to the readme. This great, thanks Mark 🎉

python/cuproj/README.md Outdated Show resolved Hide resolved
python/cuproj/README.md Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the conda Related to conda and conda configuration label Aug 2, 2023
Copy link
Contributor

@thomcom thomcom left a comment

Choose a reason for hiding this comment

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

Checks out.

Copy link
Contributor

@isVoid isVoid left a comment

Choose a reason for hiding this comment

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

LGTM, there are a few comments when visually checking the rendered doc sites.

  • In the user guide of python doc, it would be nice to time the 10000 point code and print the result, to justify the point of "really shines".
  • The developement guide can be expanded to mention how to add a feature for cuproj. As well as the inner workings of cuprojshim. But this can be a follow up.
  • This page seems a bit broken but we can fix it later since it's minor. (There are 2 Types item in the list).

jarmak-nv added a commit to jarmak-nv/cuspatial that referenced this pull request Aug 3, 2023
Co-authored-by: Ben Jarmak <104460670+jarmak-nv@users.noreply.github.com>
@harrism
Copy link
Member Author

harrism commented Aug 3, 2023

/merge

rapids-bot bot pushed a commit that referenced this pull request Aug 3, 2023
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
@isVoid isVoid requested a review from raydouglass August 3, 2023 12:33
@rapids-bot rapids-bot bot merged commit 011c5f7 into rapidsai:branch-23.08 Aug 3, 2023
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci cmake Related to CMake code or build configuration doc Documentation libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change Python Related to Python code
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

cuProj Documentation [DOC]: cuSPatial users guide (and notebook) shows version 22.08 in conda command line
7 participants