Handling duplicate points without throwing an error

[Kevin-Mattheus-Moerman]

@DanielVandH I have a question about duplicate point errors. Could DelaunayTriangulation handle the duplicate points (e.g. by removing the duplicates?) and issue a warning rather than an error?

ERROR: DuplicatePointsError: There were duplicate points. The following points are duplicated:
    (0.625, -1.55) at indices [4, 25].

[DanielVandH]

In general I prefer to make the user make the decision here. But maybe this would be nice. The identified duplicates could be passed to the existing kwarg skip_points (merged with the existing if needing). Do you want to make a pr that does that?

[EdsterG]

I generally agree with your philosophy @DanielVandH, however there is one use case where making the user deal with it is quite annoying. Specifically the Makie case, there have been many times where I wanted to visualize the data without care for duplicates. It would be super convenient to allow Makie to filter those points. With that said, Makie could in theory pipe all of the data through a unique function call, but that generally seems wasteful because most data won't have duplicates. If this library already has some logic for duplicate detection, maybe it can silently skip those points (put behind a flag). Then Makie can set that flag accordingly?

[DanielVandH]

I think changing the default behaviour to skipping duplicates and displaying a warning to the user is sufficient. Making this a flag that Makie sets would be annoying I think since I don't think I want to make this a new keyword argument to triangulate (I could make this something I do in v2 of the package where I make these changes a lot less annoying - a v2 branch exists but haven't gotten to it in a while), and the alternative is making a flag similar to

DelaunayTriangulation.jl/src/setup.jl

Line 42 in 9134c8e

const INF_WARN = Ref(true)

that Makie sets and resets which might be a bit awkward. Does the warning need to be possible to silence?

My suggestion in my last reply would be pretty simple to implement I think.

DelaunayTriangulation.jl/src/algorithms/triangulation/main.jl

Line 154 in 9134c8e

check_arguments && check_args(points, boundary_nodes, full_polygon_hierarchy, boundary_curves)

would be changed to check_arguments && (skip_points = check_args(points, boundary_nodes, full_polygon_hierarchy, boundary_curves, skip_points), and the code here https://github.com/JuliaGeometry/DelaunayTriangulation.jl/blob/main/src/algorithms/triangulation/check_args.jl would be modified accordingly, e.g.

DelaunayTriangulation.jl/src/algorithms/triangulation/check_args.jl

Line 26 in 9134c8e

has_unique_points(points)

becomes skip_points = has_unique_points(points, skip_points) and you make use of

DelaunayTriangulation.jl/src/algorithms/triangulation/check_args.jl

Line 57 in 9134c8e

dup_seen = find_duplicate_points(points)

to figure out what points to merge into the existing skip_points (skip_points can probably be converted into a Set so that mutation works regardless of what the user provides, be it a Tuple or a Vector). Here

DelaunayTriangulation.jl/src/algorithms/triangulation/check_args.jl

Line 121 in 9134c8e

!all_unique && throw(DuplicatePointsError(points))

you would warn instead of throw.

[EdsterG]

Does the warning need to be possible to silence?

I would vote yes, because inline warnings in a notebook can be quite annoying, since they mess with the formatting of the output cell.

[EdsterG]

Out of curiosity, for non-plotting use cases why is it important to warn/error on duplicate points?

[DanielVandH]

Ah okay, I don't use notebooks often so I'm not familiar. I'm happy to have a flag similar to the INF_WARN I linked then. Makie wouldn't be the one to set this though, that would be the user. See

DelaunayTriangulation.jl/src/algorithms/voronoi/unbounded.jl

Line 38 in 9134c8e

if any(isinf, (cx, cy)) && INF_WARN[]

DelaunayTriangulation.jl/src/setup.jl

Line 42 in 9134c8e

const INF_WARN = Ref(true)

[DanielVandH]

Out of curiosity, for non-plotting use cases why is it important to warn/error on duplicate points?

At least in my use cases, I am typically always interested in knowing when I have duplicate points since that indicates something wrong is occurring. e.g. for cell biology if I have two-overlapping cells then something's gone wrong in the simulation and any future simulations would probably be nonsense.

I also just like the philosophy of forcing the user to make a decision, though I recognise that can be a bit annoying especially now that Makie uses the package - ideally users just trying to plot shouldn't even really have to be aware that this package exists.

[DanielVandH]

v1.6.4 will now have a fix for this @Kevin-Mattheus-Moerman @EdsterG .

[EdsterG]

Yeah, this is great. Thanks for the fix!