-
Notifications
You must be signed in to change notification settings - Fork 47
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
K-NN Regressor #268
K-NN Regressor #268
Conversation
I would prefer that we all agree on this before merging the pull request. |
I don't have an opinion. But it is always easier to relax the constraints later (i.e. allow only rank 2 for now and potentially also rank 1 later). @msluszniak do you have any thoughts? |
Yes, I agree that for now we can accept only 2-dim labels |
Alright, will push the code with changes soon. :) |
Beautiful, thank you @krstopro! Once @msluszniak updates both KNN guides we will ship a new version!!! |
Implements k-NN regressor model and removes previous implementation of k-NN.
As usual there are few things I am not sure about:
KNNClassifier
(e.g.deftransformp compute_knn
). Was thinking pulling it out of both modules and putting it somewhere else, but that might make the code harder to understand.y
can be both rank-1 and rank-2. I think this is slightly different than usual, where tensors that can contain more than one feature are always rank-2. I would suggest allowingy
to be only of rank 2.Closes #255.