Add NVBench and linestring distance benchmark #577
Conversation
…into fea/bench_linestring_distance
…into fea/bench_linestring_distance
github-actions
bot
added
cmake
isVoid
added
feature request
|
Tagging @vyasr for cmake related changes, thanks! |
| thrust::make_counting_iterator(static_cast<cudf::size_type>(num_points)), | ||
| offsets.begin(), | ||
| [num_segments_per_string] __device__(auto i) { return i * num_segments_per_string; }); | ||
| auto points_x = cudf::make_fixed_width_column( |
There was a problem hiding this comment.
Why deal with columns at all? This forces you to deal with x and y separately when you could do them both with the same functor and reduce the number of Thrust calls nearly in half. That would make this code much more readable without comments and also more maintainable.
There was a problem hiding this comment.
Hmm, now that you mention, you are right about that. The code is adapted from an earlier version that uses the cudf column API. With header only API in place, we can bypass using columns in all.
…into fea/bench_linestring_distance
|
|
||
| #include <memory> | ||
|
|
||
| namespace cuspatial { |
There was a problem hiding this comment.
Not sure why these functions are being added to the cuSpatial namespace?
There was a problem hiding this comment.
Good catch. The benchmarks shouldn't be counted in the library namespace because they are considered external code that treats the target API as a black box. I think it would be helpful to mention this in https://github.com/rapidsai/cudf/blob/branch-22.08/cpp/docs/BENCHMARKING.md
There was a problem hiding this comment.
Did you open an issue for adding a namespace mention to BENCHMARKING.md?
|
rerun tests |
1 similar comment
|
rerun tests |
|
@gpucibot merge |
This PR adds NVBench to cuspatial and introduces benchmarks for
pairwise_linestring_distance.