{"action":"created","issue":{"url":"https://api.github.com/repos/mlpack/mlpack/issues/672","repository_url":"https://api.github.com/repos/mlpack/mlpack","labels_url":"https://api.github.com/repos/mlpack/mlpack/issues/672/labels{/name}","comments_url":"https://api.github.com/repos/mlpack/mlpack/issues/672/comments","events_url":"https://api.github.com/repos/mlpack/mlpack/issues/672/events","html_url":"https://github.com/mlpack/mlpack/issues/672","id":158370173,"number":672,"title":"Resetting stats in NeighborSearch.","user":{"login":"MarcosPividori","id":4282009,"avatar_url":"https://avatars.githubusercontent.com/u/4282009?v=3","gravatar_id":"","url":"https://api.github.com/users/MarcosPividori","html_url":"https://github.com/MarcosPividori","followers_url":"https://api.github.com/users/MarcosPividori/followers","following_url":"https://api.github.com/users/MarcosPividori/following{/other_user}","gists_url":"https://api.github.com/users/MarcosPividori/gists{/gist_id}","starred_url":"https://api.github.com/users/MarcosPividori/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MarcosPividori/subscriptions","organizations_url":"https://api.github.com/users/MarcosPividori/orgs","repos_url":"https://api.github.com/users/MarcosPividori/repos","events_url":"https://api.github.com/users/MarcosPividori/events{/privacy}","received_events_url":"https://api.github.com/users/MarcosPividori/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2016-06-03T13:42:47Z","updated_at":"2016-06-03T15:13:48Z","closed_at":null,"body":"In NeighborSearch, when the method Search(Tree* queryTree, ...) uses the given tree, it is assuming that stats there were intialized to their initial values.\r\nShouldn't we add a comment about this to the documentation?\r\nI mean, if we do: \r\n\r\n  KNN knn; \r\n  KNN::Tree queryTree(querySet);\r\n  knn.Train(someReferenceSet); \r\n  knn.Search(queryTree,.....); \r\n  knn.Train(someDifferentReferenceSet);\r\n\r\nThe last search will probably give incorrect results, because the queryTree holds old unrelated statistic values for the bounds. I mean, it uses bound values from the first referenceSet to search in the second referenceSet."},"comment":{"url":"https://api.github.com/repos/mlpack/mlpack/issues/comments/223606789","html_url":"https://github.com/mlpack/mlpack/issues/672#issuecomment-223606789","issue_url":"https://api.github.com/repos/mlpack/mlpack/issues/672","id":223606789,"user":{"login":"rcurtin","id":1845039,"avatar_url":"https://avatars.githubusercontent.com/u/1845039?v=3","gravatar_id":"","url":"https://api.github.com/users/rcurtin","html_url":"https://github.com/rcurtin","followers_url":"https://api.github.com/users/rcurtin/followers","following_url":"https://api.github.com/users/rcurtin/following{/other_user}","gists_url":"https://api.github.com/users/rcurtin/gists{/gist_id}","starred_url":"https://api.github.com/users/rcurtin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rcurtin/subscriptions","organizations_url":"https://api.github.com/users/rcurtin/orgs","repos_url":"https://api.github.com/users/rcurtin/repos","events_url":"https://api.github.com/users/rcurtin/events{/privacy}","received_events_url":"https://api.github.com/users/rcurtin/received_events","type":"User","site_admin":false},"created_at":"2016-06-03T15:13:48Z","updated_at":"2016-06-03T15:13:48Z","body":"I think I have gotten myself confused again.  You are right, there is actually no issue with `treeNeedsReset` since that can only happen for the case of monochromatic search.  So the bug I thought existed, does not exist.\r\n\r\nI was not thinking about the overload where the user passes their own query tree; in that case, we have no need to reset the query tree.  I think that we should add documentation ensuring that the user passes a tree with reset bounds, instead of resetting it for them, because (a) that will slow down the `Search()` method and (b) this is already the \"advanced\" way to call `Search()`, so if users want to build their own special tree that possibly has non-reset bounds for their own reasons, we should allow them to do that.\r\n\r\nSo in short, I agree with your original statement, that we should add documentation, but I got confused and sidetracked while I was reading through trying to understand. :)"}}
{"actor":{"display_login":"rcurtin"}}