arvkevi / kneed

Knee point detection in Python :chart_with_upwards_trend:
https://kneed.readthedocs.io
BSD 3-Clause "New" or "Revised" License
734 stars 74 forks source link

Issue in the way all_knees and all_elbows are ordered #161

Open leonfodoulian opened 6 months ago

leonfodoulian commented 6 months ago

Hi,

I think there is an issue in the way the knees and elbows are ordered when online = True. I am using the kneed package in R. And the code below is reproducible:

# import kneed as kn
kn <- reticulate::import(module = "kneed")

# Compute kernel density estimates of mtcars$mpg
dens <- density(x = mtcars$mpg,
                from = 19,
                to = 45)

# Identify knees in the density distribution
kneed.res <- kn$KneeLocator(x = dens$x,
                            y = dens$y,
                            S = 1,
                            curve = "convex",
                            direction = "decreasing",
                            online = TRUE)

# Knee and elbow values are not ordered
kneed.res$all_knees
# {26.93737769080235, 19.0, 36.50293542074364}
kneed.res$all_elbows
# {26.93737769080235, 19.0, 36.50293542074364}

I have to mention that this is not a general observation, but I found it occurring in many of my implementations of the function.

Best, Leon

leonfodoulian commented 6 months ago

I forgot to mention that because of this behaviour all_knees and all_knees_y do not match, as the latter is ordered as expected.

arvkevi commented 6 months ago

Good catch. all_knees is a Python set(), which is unordered. I don't know why I chose to use a set there. It's old code at this point. I'll take a look and probably convert it to an array. Thanks!