Github user jkbradley commented on the issue:
https://github.com/apache/spark/pull/15148
* Do we want to use the subpackage ```spark.ml.feature.lsh``` or just put
the classes under ```spark.ml.feature```? This would be the first division of
```feature```. I'd prefer not using subpackage ```lsh``` to be consistent.
> (MLnick) I can see for binary (i.e. hamming dist) that Array[Boolean] is
attractive as a kind of type safety thing, but still I think a Vector interface
is more natural.
We could allow both, though that would require changing the LSH
abstraction. In the future, I do hope ML algorithms become more relaxed in
terms of which Catalyst types they accept.
> (MLnick) efficiently support top-k recommendations across an entire
dataset
I like the idea of returning results with the top-k values since I agree
it's closer to what most users would want, versus specifying a threshold. I
assume it would be more expensive, requiring some grouping of the data.
Perhaps we can add it in a follow-up PR.
@Yunni Thanks for sending the PR! I'd be happy to make a more detailed
review pass, though I'll wait for some of the comments to be addressed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
