-
Notifications
You must be signed in to change notification settings - Fork 156
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
WGS84 <--> UTM projection header-only API #1191
WGS84 <--> UTM projection header-only API #1191
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very much a novice in the C++/CUDA world - a few comments, but it looks very nice! From the GIS perspective and someone who has used PyProj often, it looks like the backend I'd expect to see 🌐
I'll file an issue to expand the readme once we figure out exactly what will be making it into 23.08 (column API, Python API etc.)
This might be me failing to follow along how this works, so maybe a bunch of nonsense below:
Looking at wgs_to_utm_test.cu
we talk about (lat, lon)
pairs as the coordinate to be transformed, but as far as I can see coordinates are (x, y)
formatted where (lat, lon)
is (y, x)
formatted (never enjoyed this little "feature" of lat/lon). I think we just need to update any mention of (lat, lon)
to (lon, lat)
.
cpp/cuproj/tests/wgs_to_utm_test.cu
Outdated
// generate (lat, lon) points on a grid between -60 and 60 degrees longitude and | ||
// -40 and 80 degrees latitude | ||
int num_points_x = 100; | ||
int num_points_y = 100; | ||
coordinate<T> min_corner = {-26.5, -152.5}; | ||
coordinate<T> max_corner = {-25.5, -153.5}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the comment and the generated coord ranges disagree here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the comment. Regarding lon, lat vs. lat, lon. This is a good point. Currently I assume the data is lat, lon which is the opposite of assumptions in cuSpatial. I inject an axis_swap to convert to lon, lat, in fact, which is what PROJ does. But when the user has data that is lon, lat, that axis_swap is not correct, so this needs to be made a configuration option.
I was thinking about this -- do we want a column API? Or do we just want to deal with generic Python arrays of some sort? Decision on this will affect what the layer that provides Python binding points will look like. |
A good team question! When considering from the cuSpatial point-of-view I think a column API seems right. When considering cuProj alone - that's more interesting. A few questions, ordered in what I think is importance, since fairly unfamiliar:
|
@harrism when you say generic python array - do you mean arrays that conforms to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. This is looking very promising. I'm not pressing request changes because I'm out next week. Don't want to be the blocker of merging.
cpp/cuproj/include/cuproj/operation/clamp_angular_coordinates.cuh
Outdated
Show resolved
Hide resolved
cpp/cuproj/include/cuproj/operation/clamp_angular_coordinates.cuh
Outdated
Show resolved
Hide resolved
cpp/cuproj/include/cuproj/operation/clamp_angular_coordinates.cuh
Outdated
Show resolved
Hide resolved
cpp/cuproj/include/cuproj/operation/clamp_angular_coordinates.cuh
Outdated
Show resolved
Hide resolved
cpp/cuproj/include/cuproj/operation/clamp_angular_coordinates.cuh
Outdated
Show resolved
Hide resolved
@@ -57,7 +55,7 @@ class degrees_to_radians : operation<Coordinate> { | |||
__host__ __device__ Coordinate forward(Coordinate const& coord) const | |||
{ | |||
using T = typename Coordinate::value_type; | |||
return Coordinate{static_cast<T>(coord.x * DEG_TO_RAD), static_cast<T>(coord.y * DEG_TO_RAD)}; | |||
return Coordinate{coord.x * DEG_TO_RAD<T>, coord.y * DEG_TO_RAD<T>}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we expect other coordinate type than just cuspatial::vec_2d
? Otherwise I thought we can just do coord * DEG_TO_RAD<T>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was concerned about that not working for POD data types, for example. But perhaps I shouldn't worry and just assume proper coordinate types. We can't even use it directly with PJ_COORD because it's a union... https://proj.org/en/9.2/development/reference/datatypes.html#c.PJ_COORD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably provide an adaptor from PJ_COORD in the future.
/merge |
Closes #1215 Depends on #1191 New benchmark for C++ API for WGS84->UTM transform. Below results were run on a single CPU core for Proj (ignore gbench's claims to the contrary), and on a single H100 80GB GPU. Machine was a DGX H100. ``` (rapids) coder ➜ ~/cuspatial $ CUDA_VISIBLE_DEVICES=1 cpp/build/latest/cuproj/benchmarks/WGS_TO_UTM_BENCH 2023-08-02T00:08:52+00:00 Running cpp/build/latest/cuproj/benchmarks/WGS_TO_UTM_BENCH Run on (224 X 3800 MHz CPU s) CPU Caches: L1 Data 48 KiB (x112) L1 Instruction 32 KiB (x112) L2 Unified 2048 KiB (x112) L3 Unified 107520 KiB (x2) Load Average: 1.70, 6.21, 14.43 --------------------------------------------------------------------------------------------------------------------- Benchmark Time CPU Iterations UserCounters... --------------------------------------------------------------------------------------------------------------------- proj_utm_benchmark/forward_double/100 0.013 ms 0.013 ms 57756 items_per_second=7.95314M/s proj_utm_benchmark/forward_double/1000 0.114 ms 0.114 ms 6118 items_per_second=8.73369M/s proj_utm_benchmark/forward_double/10000 1.24 ms 1.24 ms 588 items_per_second=8.07697M/s proj_utm_benchmark/forward_double/100000 12.0 ms 12.0 ms 58 items_per_second=8.35593M/s proj_utm_benchmark/forward_double/1000000 120 ms 120 ms 6 items_per_second=8.36301M/s proj_utm_benchmark/forward_double/10000000 1213 ms 1213 ms 1 items_per_second=8.24563M/s proj_utm_benchmark/forward_double/100000000 11977 ms 11976 ms 1 items_per_second=8.35038M/s proj_utm_benchmark/forward_double/1000000000 119680 ms 119677 ms 1 items_per_second=8.35579M/s cuproj_utm_benchmark/forward_float/100/manual_time 0.011 ms 0.040 ms 66283 items_per_second=9.47605M/s cuproj_utm_benchmark/forward_float/1000/manual_time 0.012 ms 0.041 ms 56799 items_per_second=81.2019M/s cuproj_utm_benchmark/forward_float/10000/manual_time 0.013 ms 0.042 ms 55571 items_per_second=793.482M/s cuproj_utm_benchmark/forward_float/100000/manual_time 0.013 ms 0.042 ms 53048 items_per_second=7.5779G/s cuproj_utm_benchmark/forward_float/1000000/manual_time 0.027 ms 0.056 ms 25842 items_per_second=36.9063G/s cuproj_utm_benchmark/forward_float/10000000/manual_time 0.170 ms 0.198 ms 4130 items_per_second=58.8554G/s cuproj_utm_benchmark/forward_float/100000000/manual_time 1.60 ms 1.62 ms 439 items_per_second=62.6581G/s cuproj_utm_benchmark/forward_float/1000000000/manual_time 15.9 ms 15.9 ms 44 items_per_second=63.0518G/s cuproj_utm_benchmark/forward_double/100/manual_time 0.012 ms 0.041 ms 57960 items_per_second=8.30297M/s cuproj_utm_benchmark/forward_double/1000/manual_time 0.015 ms 0.044 ms 47605 items_per_second=68.0791M/s cuproj_utm_benchmark/forward_double/10000/manual_time 0.015 ms 0.044 ms 47353 items_per_second=676.864M/s cuproj_utm_benchmark/forward_double/100000/manual_time 0.016 ms 0.045 ms 43394 items_per_second=6.19684G/s cuproj_utm_benchmark/forward_double/1000000/manual_time 0.042 ms 0.070 ms 16621 items_per_second=23.7599G/s cuproj_utm_benchmark/forward_double/10000000/manual_time 0.304 ms 0.332 ms 2302 items_per_second=32.863G/s cuproj_utm_benchmark/forward_double/100000000/manual_time 2.93 ms 2.96 ms 240 items_per_second=34.087G/s cuproj_utm_benchmark/forward_double/1000000000/manual_time 29.3 ms 29.3 ms 24 items_per_second=34.13G/s Authors: - Mark Harris (https://github.com/harrism) Approvers: - Michael Wang (https://github.com/isVoid) URL: #1216
Depends on #1132
Closes #1139
Description
Implements WGS to UTM projection (either direction) in CUDA C++. This is based on the code from PROJ, and that library is used for comparison in the tests.
To reviewers, I would start by looking over the test: wgs_to_utm_test.cu. This uses
projection_factories.hpp
, so review that next, then look atprojection.cuh
andprojection_parameters.cuh
. The projection pipeline is applied using athrust::transform()
where the operator is apipeline
made up of a sequence ofoperation
s (operation.cuh). A pipeline can be applied forward or inverse. The operations for UTM projection are all in include/cuproj/operation.Note that the dispatch of operations is a little clunky (switch statement in a device function). This is due to limitations of creating arrays of objects on the host for virtual dispatch on the device. This bears more experimentation in the future to come up with a better approach, but this works fine for now.
TODO:Benchmark and Python API are left to follow-up PRs.
Checklist