JuliaGeometry / VoronoiDelaunay.jl

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

tests for counting edges #57

Closed Wikunia closed 2 years ago

Wikunia commented 2 years ago

Before continuing with #53 I think it would be good to have these test cases. I assumed that the number is correct here just wanted to add a test such that we see when this changes. Not sure how to better test this.

dkarrasch commented 2 years ago

Awesome. Could you please rebase onto current master, so that the github action CI stuff runs on this PR.

You could try and see if something like length(voronoiedges(tess)) works and is equal to the result of your loop. That should be an independent "stepping through" test. Could you please also fix the random seed in the "point can be in only one triangle" test. That test turned out to give spurious errors, probably when the test point was lying on some edge.

dkarrasch commented 2 years ago

With #53 we won't need the collect in the tests anymore, right?

codecov[bot] commented 2 years ago

Codecov Report

Merging #57 (235cf75) into master (63898ec) will increase coverage by 15.05%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #57       +/-   ##
===========================================
+ Coverage   72.20%   87.26%   +15.05%     
===========================================
  Files           1        1               
  Lines         421      424        +3     
===========================================
+ Hits          304      370       +66     
+ Misses        117       54       -63     
Impacted Files Coverage Δ
src/VoronoiDelaunay.jl 87.26% <0.00%> (+15.05%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 63898ec...235cf75. Read the comment docs.

Wikunia commented 2 years ago

Yes that might be true though I'm not 100% how iterators handle length :smile:

Wikunia commented 2 years ago

Seems like there is a difference in the delaunay edges between Julia 1.6 and 1.7 though? :confused:

dkarrasch commented 2 years ago

Hm, I believe we've had changes in the random number generator between v1.6 and v1.7 or so. We should perhaps try to avoid hard-coding numbers, and instead compare two different implementations that we expect to give consistent results.

Wikunia commented 2 years ago

Oh yeah that's true maybe have a file with a list of points that we can read and use those as our test case?

dkarrasch commented 2 years ago

We don't need a big test. Perhaps you could manually type a couple of points, say 5, at distinct locations, and then we know everything about that specific case. It is only important to step through the iterator, even if its length is short.

Wikunia commented 2 years ago

True, will do

Wikunia commented 2 years ago

Some lines have a length of zero though:

image

Here you can see the 12 visible voronoi edges.

Wikunia commented 2 years ago

With #53 we won't need the collect in the tests anymore, right?

Actually we do because we don't provide the length of the iterator at the moment. We could walk through all and count and return that though if we want to.