JuliaGeometry / VoronoiDelaunay.jl

Fast and robust Voronoi & Delaunay tessellation creation with Julia
Other
123 stars 26 forks source link

Iterator for voronoiedges #53

Closed mikkoku closed 2 years ago

mikkoku commented 3 years ago

Change voronoiedges to return an iterator instead of a Channel.

In my use case this results in a 10-100x faster iteration over voronoiedges.

Wikunia commented 2 years ago

I can confirm that a Channel seems extremely slow in my use case. What needs to be done to get this merged? Is this project still maintained?

dkarrasch commented 2 years ago

Can someone give a little piece of code so that I can test for performance. Also, is this fully tested?

Wikunia commented 2 years ago

Thanks for the fast reply. Sure will create a small example for you. 😊 Regarding testing: I imagined the same tests should run that's why I'm a bit confused by the zero coverage on this PR but I'm happy to look into it. Just wanted to check first whether there is interested in this.

dkarrasch commented 2 years ago

Yes, if the interface doesn't change and we gain performance, there's interest, even though it hadn't looked like it since October 2020. Sorry for that. Maybe I'll need to update the test and coverage infrastructure on master, and then may ask @mikkoku to rebase. I'll get back to you ASAP.

dkarrasch commented 2 years ago

On master, this iterator seems to be untested, and so will be the modified version, see my #56. So I have to kind requests: first, could you please rewrite the "withoutgenerator" version in the same way, and second, could you please add some test, perhaps similar to the example in the README (i += 1, counting the edges, nothing fancy) for a small controllable example. That would make sure that the iterator goes through.

Wikunia commented 2 years ago

I created a fork https://github.com/Wikunia/VoronoiDelaunay.jl/tree/feature-iterator-instead-of-channel and branch there for this where you can checkout the test/current.jl file which creates n random points (like your readme) and then iterates over voronoi edges.

Here are two benchmark results for the iterate_over_tess_channel and iterate_over_tess_iterator functions for n = 100 Both result in i == 301 in the end.

julia> @benchmark iterate_over_tess_channel(tess)
BenchmarkTools.Trial: 3018 samples with 1 evaluation.
 Range (min … max):  1.480 ms …   4.860 ms  ┊ GC (min … max): 0.00% … 65.36%
 Time  (median):     1.647 ms               ┊ GC (median):    0.00%
 Time  (mean ± σ):   1.653 ms ± 109.920 μs  ┊ GC (mean ± σ):  0.17% ±  2.00%
 Memory estimate: 48.70 KiB, allocs estimate: 626.

julia> @benchmark iterate_over_tess_iterator(tess)
BenchmarkTools.Trial: 10000 samples with 6 evaluations.
 Range (min … max):  5.285 μs …   8.555 μs  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     5.564 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   5.572 μs ± 139.950 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%
 Memory estimate: 0 bytes, allocs estimate: 0.

Which in this case is a speedup of about 300 and no allocations is of course always good :)

I can try to rewrite the without generator function in the same way but probably later today or tmr. Shall I open a new PR here then?