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

Improve C++ header file organization #1080

Closed
Tracked by #663
harrism opened this issue Apr 18, 2023 · 0 comments · Fixed by #1097
Closed
Tracked by #663

Improve C++ header file organization #1080

harrism opened this issue Apr 18, 2023 · 0 comments · Fixed by #1097
Assignees
Labels
improvement Improvement / enhancement to an existing function libcuspatial Relates to the cuSpatial C++ library

Comments

@harrism
Copy link
Member

harrism commented Apr 18, 2023

No description provided.

@harrism harrism self-assigned this Apr 18, 2023
@harrism harrism added improvement Improvement / enhancement to an existing function libcuspatial Relates to the cuSpatial C++ library labels Apr 18, 2023
@harrism harrism modified the milestone: header-only C++ API Apr 18, 2023
@harrism harrism moved this from Todo to In Progress in cuSpatial Apr 24, 2023
@jarmak-nv jarmak-nv moved this from In Progress to Review in cuSpatial May 1, 2023
@rapids-bot rapids-bot bot closed this as completed in #1097 May 2, 2023
rapids-bot bot pushed a commit that referenced this issue May 2, 2023
Fixes #1080 based on the discussion in #1087

-  Consolidate each major set of features within common headers, e.g. `distance.hpp/.cuh`, `intersection.hpp/.cuh`.
-  Consolidate point-in-polygon APIs / headers and DRY up implementation
- Rename some headers/APIS that were inconsistent across the header-only and column-based APIs, e.g. `points_in_spatial_window` vs. `points_in_range`.
- Update Cython where needed to account for above changes.

This PR does NOT yet reorganize source code.

Update: Compilation Time Results
---------------------------------------------

Compilation time is not changed appreciably by this PR (<0.5% faster).

This test was performed by disabling `sccache`, erasing everything in the build directory, configuring cmake (including building tests and benchmarks), and then running `time ninja`. My machine has a "AMD Ryzen 7 3700X 8-Core Processor", and I'm using Ninja's default parallelism.

#### Disabling `sccache` when using the cuSpatial devcontainer

```
(rapids) coder ➜ ~/cuspatial/ $ cd cpp/build/release
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ rm -rf ./*
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ unset RUSTC_WRAPPER
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ unset CMAKE_C_COMPILER_LAUNCHER
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ unset CMAKE_CXX_COMPILER_LAUNCHER
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ unset CMAKE_CUDA_COMPILER_LAUNCHER
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ cmake ~/cuspatial/cpp -GNinja -DBUILD_TESTS=ON -DBUILD_BENCHMARKS=ON
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ time ninja
```

### This branch: 

```
[201/201] Linking CXX executable gtests/LINESTRING_INTERSECTION_TEST_EXP

real    10m42.845s
user    122m28.077s
sys     6m7.935s
```

### `branch-23.06`:

```
[202/202] Linking CXX executable gtests/LINESTRING_INTERSECTION_TEST_EXP

real    10m45.573s
user    122m52.896s
sys     6m9.357s
```

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

Approvers:
  - Michael Wang (https://github.com/isVoid)
  - Paul Taylor (https://github.com/trxcllnt)
  - H. Thomson Comer (https://github.com/thomcom)

URL: #1097
@github-project-automation github-project-automation bot moved this from Review to Done in cuSpatial May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function libcuspatial Relates to the cuSpatial C++ library
Projects
Status: Done
1 participant