{"action":"created","issue":{"url":"https://api.github.com/repos/jlmelville/uwot/issues/15","repository_url":"https://api.github.com/repos/jlmelville/uwot","labels_url":"https://api.github.com/repos/jlmelville/uwot/issues/15/labels{/name}","comments_url":"https://api.github.com/repos/jlmelville/uwot/issues/15/comments","events_url":"https://api.github.com/repos/jlmelville/uwot/issues/15/events","html_url":"https://github.com/jlmelville/uwot/issues/15","id":389064424,"node_id":"MDU6SXNzdWUzODkwNjQ0MjQ=","number":15,"title":"C++ (and other) code review","user":{"login":"LTLA","id":8166669,"node_id":"MDQ6VXNlcjgxNjY2Njk=","avatar_url":"https://avatars3.githubusercontent.com/u/8166669?v=4","gravatar_id":"","url":"https://api.github.com/users/LTLA","html_url":"https://github.com/LTLA","followers_url":"https://api.github.com/users/LTLA/followers","following_url":"https://api.github.com/users/LTLA/following{/other_user}","gists_url":"https://api.github.com/users/LTLA/gists{/gist_id}","starred_url":"https://api.github.com/users/LTLA/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/LTLA/subscriptions","organizations_url":"https://api.github.com/users/LTLA/orgs","repos_url":"https://api.github.com/users/LTLA/repos","events_url":"https://api.github.com/users/LTLA/events{/privacy}","received_events_url":"https://api.github.com/users/LTLA/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2018-12-09T21:09:23Z","updated_at":"2018-12-10T10:01:00Z","closed_at":null,"author_association":"NONE","body":"I thought I'd go one by one over each of the C++ files, starting with `nn_parallel.h`. It seems you're doing a nearest-neighbor search on `X` in the R `annoy_nn` function. Here's some observations:\r\n\r\n- Annoy will return the observation itself as its own nearest neighbor. I usually have to search `k+1` neighbors and check if the observation is not its own neighbor, see [here](https://github.com/LTLA/BiocNeighbors/blob/master/src/find_annoy.cpp).\r\n- On that note, you can also use the `get_nns_by_item` method for get the NNs for a particular item in the index. This avoids the need to repass `mat` into the `NNworker`, and eliminates the row-by-row accesses that are not cache optimal in `NNworker::operator()`. \r\n- You may have some small-to-moderate efficiency gains by saving output in columns of `idx` and `dists`, which would be more cache optimal. That is, create transposed matrices for `NNWorker` to dump results in, and untranspose them just before or after you return to R.\r\n- Probably the default grain size should be larger, to avoid false sharing and the like?"},"comment":{"url":"https://api.github.com/repos/jlmelville/uwot/issues/comments/445757851","html_url":"https://github.com/jlmelville/uwot/issues/15#issuecomment-445757851","issue_url":"https://api.github.com/repos/jlmelville/uwot/issues/15","id":445757851,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NTc1Nzg1MQ==","user":{"login":"LTLA","id":8166669,"node_id":"MDQ6VXNlcjgxNjY2Njk=","avatar_url":"https://avatars3.githubusercontent.com/u/8166669?v=4","gravatar_id":"","url":"https://api.github.com/users/LTLA","html_url":"https://github.com/LTLA","followers_url":"https://api.github.com/users/LTLA/followers","following_url":"https://api.github.com/users/LTLA/following{/other_user}","gists_url":"https://api.github.com/users/LTLA/gists{/gist_id}","starred_url":"https://api.github.com/users/LTLA/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/LTLA/subscriptions","organizations_url":"https://api.github.com/users/LTLA/orgs","repos_url":"https://api.github.com/users/LTLA/repos","events_url":"https://api.github.com/users/LTLA/events{/privacy}","received_events_url":"https://api.github.com/users/LTLA/received_events","type":"User","site_admin":false},"created_at":"2018-12-10T10:01:00Z","updated_at":"2018-12-10T10:01:00Z","author_association":"NONE","body":"My guess is that numba's parallelization is very low level. Probably occurs only for simple operations that are known to be vectorizable, e.g., calculation of the Euclidean distance."}}
{"actor":{"display_login":"LTLA"}}