Skip to content
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/DiscreteVoronoi.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ using Random: shuffle
using StaticArrays
using Distances: euclidean

Coord = SVector{2,Int}
const Coord = SVector{2,Int}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thumbs up


# Completely non-exported files with core definitions
include("EarlyStopper.jl") # Because every package needs at least one struct...
Expand Down
66 changes: 44 additions & 22 deletions src/core_algorithms.jl
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,28 @@ end
elseif length(sites) == 1 # Same if there is a single site
view(grid, TL[1]:BR[1], TL[2]:BR[2]) .= Ref(first(sites))
else
# Otherwise we check if all corners have the same closest site
# Otherwise we check if all corners have the same closest site ...

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part I don't understand

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you modify the code here like

            if all(zip(mins, dists)) do ((min_dist, _), dist)
                    dist > min_dist
                end
                grid[TL[1]:BR[1], TL[2]:BR[2]] .= Ref(min_site)
                return nothing
            else
                @show (distance, mins, dists)
                # grid[TL[1]:BR[1], TL[2]:BR[2]] .= Ref(min_site)
                return nothing
            end

you'll see the non unique sites and if you also set the rectangle to the the site you'll see the test failures for Chebyshev distance. BTW we could also drop the whole code sequence, because filtering is good enough (see the benchmark for different find and filter combinations in legacy).

corners = get_corners(TL, BR)
closest_corners = (find_closest_site(corner, sites, distance=distance) for corner in corners)
if allequal(closest_corners)
grid[TL[1]:BR[1], TL[2]:BR[2]] .= Ref(first(closest_corners))
else
# And if not we divide the grid into quadrants and "conquer" each one
for (quadrant_TL, quadrant_BR) in get_quadrants(TL, BR)
_dac_voronoi!(grid, quadrant_TL, quadrant_BR, sites; distance=distance)
mins = ((findmin(sites) do site
distance(corner, site)
end for corner in corners)...,)
if allequal(site for (dist, site) in mins)
# ... and if the closest site is unique
min_site = sites[mins[1][2]]
dists = ((minimum(site for site in sites if site != min_site) do site
distance(corner, site)
end for corner in corners)...,)
if all(zip(mins, dists)) do ((min_dist, _), dist)
dist > min_dist
end
grid[TL[1]:BR[1], TL[2]:BR[2]] .= Ref(min_site)
return nothing
end
end
# And if not we divide the grid into quadrants and "conquer" each one
for (quadrant_TL, quadrant_BR) in get_quadrants(TL, BR)
_dac_voronoi!(grid, quadrant_TL, quadrant_BR, sites; distance=distance)
end
end
return nothing
end
Expand All @@ -93,32 +104,43 @@ step which aims to reduce the work of subsequent steps.
function redac_voronoi!(grid::Matrix{T}, sites::Vector{T}; distance=euclidean, auxiliary=exact_aux) where {T<:Coord}
TL = 1, 1
BR = size(grid)
_redac_voronoi!(grid, TL, BR, EarlyStopper(sites); distance=distance, auxiliary=auxiliary)
_redac_voronoi!(grid, TL, BR, sites; distance=distance, auxiliary=auxiliary)
return nothing
end

@inbounds function _redac_voronoi!(grid, TL, BR, sites::ES; distance, auxiliary) where {ES<:EarlyStopper}
@inbounds function _redac_voronoi!(grid, TL, BR, sites; distance, auxiliary)
any(TL .> BR) && return nothing
# Then, if the grid is a single cell then we are done
if all(BR .== TL)
find_closest_site!(grid, TL, sites; distance=distance)
elseif length(sites) == 1 # Same if there is a single site
view(grid, TL[1]:BR[1], TL[2]:BR[2]) .= Ref(first(sites))
else
# Otherwise we check if all corners have the same closest site
# Otherwise we check if all corners have the same closest site ...
corners = get_corners(TL, BR)
closest_corners = (find_closest_site!(grid, corner, sites, distance=distance) for corner in corners)
if allequal(closest_corners)
view(grid, TL[1]:BR[1], TL[2]:BR[2]) .= Ref(first(closest_corners))
else
# And if not we eliminate faraway seeds from subsequent steps
# `auxiliary` sorts sites by whether the predicate is true and stores how many are true.
local_sites = auxiliary(grid, sites, TL, BR; distance=distance)
# then divide the grid into quadrants and "conquer" each one
for (quadrant_TL, quadrant_BR) in get_quadrants(TL, BR)
_redac_voronoi!(grid, quadrant_TL, quadrant_BR, local_sites; distance=distance, auxiliary=auxiliary)
mins = ((findmin(sites) do site
distance(corner, site)
end for corner in corners)...,)
if allequal(site for (dist, site) in mins)
# ... and if the closest site is unique
min_site = sites[mins[1][2]]
dists = ((minimum(site for site in sites if site != min_site) do site
distance(corner, site)
end for corner in corners)...,)
if all(zip(mins, dists)) do ((min_dist, _), dist)
dist > min_dist
end
grid[TL[1]:BR[1], TL[2]:BR[2]] .= Ref(min_site)
return nothing
end
end
# And if not we eliminate faraway seeds from subsequent steps
# `auxiliary` sorts sites by whether the predicate is true and stores how many are true.
local_sites = auxiliary(grid, sites, TL, BR; distance=distance)
# then divide the grid into quadrants and "conquer" each one
for (quadrant_TL, quadrant_BR) in get_quadrants(TL, BR)
_redac_voronoi!(grid, quadrant_TL, quadrant_BR, local_sites; distance=distance, auxiliary=auxiliary)
end
end
return nothing
end
end
10 changes: 5 additions & 5 deletions src/elimination_methods.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,23 @@ end

function centre_anchor_condition(site, sites, TL, BR; distance=euclidean)
centre = @. TL + BR / 2
anchor = find_closest_site(centre, sites)
anchor = find_closest_site(centre, sites; distance=distance)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find, didn't realise this was bugged

corners = get_corners(TL, BR)
return any(distance(site, corner) <= distance(anchor, corner) for corner in corners)
end

function centre_anchor_aux(grid, sites, TL, BR; distance=euclidean)
centre = @. TL + BR / 2
anchor = find_closest_site(centre, sites)
anchor = find_closest_site(centre, sites; distance=distance)
corners = get_corners(TL, BR)
function predicate(site)
return all(TL .<= site .<= BR) ||
any(distance(corner, site) <= distance(corner, anchor) for corner in corners)
end
return early_stop_sort!(sites, predicate)
return unstable_partition!(predicate, sites)[1]
end

function exact_aux(grid, sites, TL, BR; distance=euclidean)
predicate(site) = all(TL .<= site .<= BR) || exact_condition(site, sites, TL, BR; distance=distance)
return early_stop_sort!(sites, predicate)
end
return unstable_partition!(predicate, sites)[1]
end
29 changes: 29 additions & 0 deletions src/helper_functions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,35 @@ function swap!(v, i::Int, j::Int)
return nothing
end

function unstable_partition!(f, A)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you benchmark the use of unstable_partition vs EarlyStopper? I'm up for switching because it's simpler, but performance is important.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark added. To improve the last test cases we probably need something like preset_voronoi from the legacy branch.

Results from my system with 1.8.2

(n, s) = (400, 20)
jfa_voronoi!
  41.130 ms (0 allocations: 0 bytes)
dac_voronoi!
  3.201 ms (0 allocations: 0 bytes)
redac_voronoi!
exact_aux = DiscreteVoronoi.exact_aux
  1.085 ms (0 allocations: 0 bytes)
centre_anchor_aux = DiscreteVoronoi.centre_anchor_aux
  1.056 ms (0 allocations: 0 bytes)
redac_voronoi_es!
exact_aux_es = DiscreteVoronoi.exact_aux_es
  979.600 μs (0 allocations: 0 bytes)
centre_anchor_aux_es = DiscreteVoronoi.centre_anchor_aux_es
  946.800 μs (0 allocations: 0 bytes)
(n, s) = (400, 400)
jfa_voronoi!
  48.407 ms (0 allocations: 0 bytes)
dac_voronoi!
  254.303 ms (0 allocations: 0 bytes)
redac_voronoi!
exact_aux = DiscreteVoronoi.exact_aux
  7.985 ms (0 allocations: 0 bytes)
centre_anchor_aux = DiscreteVoronoi.centre_anchor_aux
  6.368 ms (0 allocations: 0 bytes)
redac_voronoi_es!
exact_aux_es = DiscreteVoronoi.exact_aux_es
  7.415 ms (0 allocations: 0 bytes)
centre_anchor_aux_es = DiscreteVoronoi.centre_anchor_aux_es
  5.867 ms (0 allocations: 0 bytes)
(n, s) = (400, 8000)
jfa_voronoi!
  64.304 ms (0 allocations: 0 bytes)
redac_voronoi!
exact_aux = DiscreteVoronoi.exact_aux
  206.388 ms (0 allocations: 0 bytes)
centre_anchor_aux = DiscreteVoronoi.centre_anchor_aux
  51.430 ms (0 allocations: 0 bytes)
redac_voronoi_es!
exact_aux_es = DiscreteVoronoi.exact_aux_es
  199.030 ms (0 allocations: 0 bytes)
centre_anchor_aux_es = DiscreteVoronoi.centre_anchor_aux_es
  50.399 ms (0 allocations: 0 bytes)
(n, s) = (400, 160000)
jfa_voronoi!
  49.841 ms (0 allocations: 0 bytes)
redac_voronoi!
exact_aux = DiscreteVoronoi.exact_aux
  7.645 s (0 allocations: 0 bytes)
centre_anchor_aux = DiscreteVoronoi.centre_anchor_aux
  686.620 ms (0 allocations: 0 bytes)
redac_voronoi_es!
exact_aux_es = DiscreteVoronoi.exact_aux_es
  7.466 s (0 allocations: 0 bytes)
centre_anchor_aux_es = DiscreteVoronoi.centre_anchor_aux_es
  707.790 ms (0 allocations: 0 bytes)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more remark: to replace _findmin with Base.findmin, EarlyStopper needs to support a keys method. Do you know how to implement it?

i, j = 1, length(A)
@inbounds while true
while i < j && f(A[i])
i += 1
end
while i < j && !f(A[j])
j -= 1
end
if i < j
A[i], A[j] = A[j], A[i]
i += 1
j -= 1
else
break
end
end
if i == j
if f(A[i])
@views return A[1:i], A[(i+1):length(A)]
else
@views return A[1:(i-1)], A[i:length(A)]
end
else
@assert i == j + 1
@views return A[1:j], A[i:length(A)]
end
end

"""
get_quadrants(TL, BR)

Expand Down