-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convex Hull fails to create for co-linear points #32
Comments
Yes, I would like to see it. The 2D version of the code is a special case and was the last thing that I added. It is meant to be fast but this is important to check. However, if someone is doing 1000+ points, then I wouldn't want this to slow things down especially since the probability that they would all be co-linear would be highly unlikely. So, if the check doesn't add much additional overhead, I would like to include it in the package. |
Yep, that's what we're adding, checking pairs of vectors against the first pair and bailing at the first non-equal result. For clarity, I'm in the process of writing it for our lib which uses its own |
Well, I found an easy place to fix this. There was already a function called "FindIntermediatePointsForLongSkinny" which I was able to make a small modification to so that it would return the two extrema along the line instead of crashing. For the general case, this adds no extra time. thanks for catching this! |
Nice one 👏 ! Does that mean the will be a point release that will support making convex hulls from co-linear points ? |
yes, the nuget is updated (https://www.nuget.org/packages/MIConvexHull/) and the details of the commit are here: 915cdb7 |
Hi @micampbell , Just tried the updated NuGet package and had 2 things to report back :
|
The second is easy to fix. The first is because the inputs are all over the place for this project. If your Vertex can inherit from MIConvexHull.IVertex2D instead of MIConvexHull.IVertex or double[], then it should work. I'm trying to get it get the API straightened out so that the other two inputs will works as well for 2D. |
Hi, first off, many thanks for the great work in this library!
We've been using it in an open-source library called RefineryToolkits and have found an issue that we wanted to discuss.
It seems that
ConvexHull
fails to be created when the input points areco-linear
. Specifically, this line of code does not throw, but returns a null when points are co-linear.Example
Below is a list of points with the coordinates that highlighted the failure during a recent hackathon :
To solve this in short-term, we are adding a co-linearity check in our library and handle that case separately, but was wondering if it's something that you'd like to address in the
MIConvexHull
library itself so everyone has access to the fix ?Let us know 👍
The text was updated successfully, but these errors were encountered: