[REVIEW] Cartesian Product Iterator

[cwharris]

Factored out the Cartesian product iterator used by the Hausdorff distance implementation so that it can be used in st distance, and other n^2 distance metrics. Also added the ability to iterate the Cartesian product of two inputs, rather than a single input against itself. One of the goals was to avoid impacting Hausdorff performance. Instead of reaching that goal, Hausdorff performance was increased by 40%. 🎉

---------------------------------------------------------------------------------------------------------------
Benchmark                                                     Time             CPU   Iterations UserCounters...
---------------------------------------------------------------------------------------------------------------
HausdorffBenchmark/hausdorff/1024/1024/manual_time        0.681 ms        0.698 ms         1022 items_per_second=1.53716G/s
HausdorffBenchmark/hausdorff/4096/1024/manual_time        0.679 ms        0.696 ms         1024 items_per_second=1.54078G/s
HausdorffBenchmark/hausdorff/16384/1024/manual_time       0.681 ms        0.698 ms         1015 items_per_second=1.53786G/s
HausdorffBenchmark/hausdorff/1024/4096/manual_time         7.33 ms         7.34 ms           95 items_per_second=2.28901G/s
HausdorffBenchmark/hausdorff/4096/4096/manual_time         9.98 ms         10.0 ms           67 items_per_second=1.67971G/s
HausdorffBenchmark/hausdorff/16384/4096/manual_time        9.97 ms         9.99 ms           68 items_per_second=1.68188G/s
HausdorffBenchmark/hausdorff/1024/32768/manual_time         531 ms          531 ms            1 items_per_second=2.02367G/s
HausdorffBenchmark/hausdorff/4096/32768/manual_time         524 ms          524 ms            1 items_per_second=2.04759G/s
HausdorffBenchmark/hausdorff/16384/32768/manual_time        538 ms          539 ms            1 items_per_second=1.99397G/s

[GPUtester]

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

[cwharris]

rerun tests

[harrism]

@cwharris please set labels and put either [WIP] or [REVIEW] in the title so we know the status.

[trxcllnt]

rerun tests

[cwharris]

This PR is needed to support development for #231.

[cwharris]

rerun tests

[trxcllnt]

Made that one suggestion to use thrust::distance, but other than that looks good to me.

[trxcllnt]

Note, style checks will be fixed once #252 is merged.

[cwharris]

rerun tests

[harrism]

One of the goals was to avoid impacting Hausdorff performance. Instead of reaching that goal, Hausdorff performance was increased by 40%.

@cwharris the benchmark you included doesn't compare two things against each other, so how am I to see a 40% increase from it? Also, why do they all say "manual_time"? (Aare you using manual timing, and if so, why?)

[cwharris]

@harrism Previous benchmarks are from #246 (comment)

I have not yet investigated why/when to use manual time, or what it means 😞. I was following the lead from other benchmarks, such as gather. Benchmark is declared here: https://github.com/rapidsai/cuspatial/blob/branch-0.15/cpp/benchmarks/hausdorff_benchmark.cpp#L56

Previous benchmarks

---------------------------------------------------------------------------------------------------------------
Benchmark                                                     Time             CPU   Iterations UserCounters...
---------------------------------------------------------------------------------------------------------------
HausdorffBenchmark/hausdorff/1024/1024/manual_time        0.801 ms        0.818 ms          869 items_per_second=1.30675G/s
HausdorffBenchmark/hausdorff/4096/1024/manual_time        0.803 ms        0.820 ms          862 items_per_second=1.30263G/s
HausdorffBenchmark/hausdorff/16384/1024/manual_time       0.804 ms        0.821 ms          854 items_per_second=1.30151G/s
HausdorffBenchmark/hausdorff/1024/4096/manual_time         10.8 ms         10.8 ms           63 items_per_second=1.54981G/s
HausdorffBenchmark/hausdorff/4096/4096/manual_time         12.3 ms         12.3 ms           56 items_per_second=1.36443G/s
HausdorffBenchmark/hausdorff/16384/4096/manual_time        12.3 ms         12.3 ms           56 items_per_second=1.36471G/s
HausdorffBenchmark/hausdorff/1024/32768/manual_time         728 ms          728 ms            1 items_per_second=1.475G/s
HausdorffBenchmark/hausdorff/4096/32768/manual_time         790 ms          790 ms            1 items_per_second=1.35852G/s
HausdorffBenchmark/hausdorff/16384/32768/manual_time        770 ms          770 ms            1 items_per_second=1.39406G/s

[jrhemstad]

The manual_time comes from using the cuda_event_timer.

[harrism]

Looks like a good way to factor out N^2 distance metrics. Most of my comments are about documentation, just a couple on code.

[thomcom]

Really great refactor, love the cartesian product iterator. I had a few questions and minor reformatting concerns but I approve, not worth holding it up over it.

[cwharris]

@thomcom did you make comments? I don't see them. 🤷‍♂️

[harrism]

Seems like there's a new undocumented and untested API added in (minimum_euclidean_distance)? If so, I think you should move it to a new PR so we can get this one merged.