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

WIP: Allow more types in the tree data #16

Closed
wants to merge 1 commit into from
Closed

Conversation

KristofferC
Copy link
Owner

This is a work in progress to resolve #13 and an alternative to #14

TODO:

  • Check for any performance regressions. Since this mess around with the type system we have to make sure that no type instability got introduced.
  • Add test cases, maybe @tlnagy can help with that.

Creating the tree works now at least:

julia> data = rand(1:4, (10, 10^4));

julia> balltree = BallTree(data, Hamming())
NearestNeighbors.BallTree{Int64,Float64,Distances.Hamming}
  Number of points: 10000
  Dimensions: 10
  Metric: Distances.Hamming()
  Reordered: true

cc @tlnagy

@tlnagy
Copy link
Contributor

tlnagy commented Dec 24, 2015

Awesome. Thanks for working on this. I'll brainstorm on a couple different test cases. Should I open a new pull request for the allow_int branch?

@tlnagy
Copy link
Contributor

tlnagy commented Dec 24, 2015

I would like to implement at least the following tests:

  • compare results on a randomly generated dataset to the distances reported by Distances.pairwise for a subset of points
  • compare results to a fixed dataset without an unambigious nearest neighbor order (i.e. multiple neighbors of the same distances away)

@KristofferC
Copy link
Owner Author

Your plan for adding tests sounds good.

I haven't done much testing with this branch on actually running any range searches with the integer tree. I just tested that I could create the tree and that I didn't break any previous functionality.

If you get the tests to work then that's great and you can open a PR against this branch. If you get problems even running basic searches then post here with a repo and I will fix it.

@KristofferC
Copy link
Owner Author

Bump, still interested in this @tlnagy?

@tlnagy
Copy link
Contributor

tlnagy commented Feb 13, 2016

I still am. Sorry, but I've been really busy recently. Will try and look into this soon.

@KristofferC
Copy link
Owner Author

Superseeded by #29

@KristofferC KristofferC deleted the allow_int branch September 12, 2016 14:09
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

Successfully merging this pull request may close these issues.

Is it possible to use integers instead of floats?
2 participants