Skip to content
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

Possible mutation-related bug #15

Open
sswatson opened this issue Apr 10, 2020 · 1 comment
Open

Possible mutation-related bug #15

sswatson opened this issue Apr 10, 2020 · 1 comment

Comments

@sswatson
Copy link

On this line, a data point is assigned to the centroids array by reference:

ks[z++] = data[idx];

This means that as the centroids change later in the algorithm, the underlying data array is also changed. This is undesirable for two reasons: mutating the data array which is passed in could have downstream effects, and even if the data array isn't used subsequently, it makes the algorithm incorrect.

Assuming I'm not missing something here (very possible, as I am not a JS developer), data[idx].slice() would be the fix I would suggest.

@solzimer
Copy link
Owner

Hi @sswatson,

This issue is related to pull request #12

The algorithm is "working as intended", in the sense of doing the things "as fast as possible". Performing this action will reduce considerably its performance, so I'm looking for a solution that resolve the issue for those that doesn't want its data mutated.

Thanks for the feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants