-
Notifications
You must be signed in to change notification settings - Fork 437
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
NF: Find the closest vertex on a sphere for an input vector. #483
Conversation
vertex (in angle). | ||
""" | ||
ang = np.arccos(np.dot(self.vertices, xyz)) | ||
ang = np.min(np.vstack([ang, np.pi - ang]), 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been using something like the following for this:
cos_sim = abs(np.dot(self.vertices, xyz))
return np.argmax(cos_sim)
If this is being done repeatedly it might be worth building a KDtree for it.
Thanks for the comments and suggestions. How about the following solution? Have each of these classes have it's own (just very slightly different) implementation of this function, taking or not taking the absolute value, as needed. I have also added a test to demonstrate that scaling by l2norm is not necessary, as long as you are not interested in the angle, but rather just in finding the index of the minimal. |
Account for antipodal symmetry as appropriate for Sphere/Hemisphere.
Just rebased on |
Will have a look first thing tomorrow morning. Sorry for the delay. |
Hi @arokem. The tests are a bit too tight because you only test finding an already existing point to itself. I would like to see in the future a test showing that if you add a new point that is equal to old point + tiny random shift you will still get the same point. But because this is a trivial problem and this PR is needed for other more complex PRs I believe is pointless to wait right now for extra testing on this and will go ahead and merge. Inf+!!! |
NF: Find the closest vertex on a sphere for an input vector.
This is useful for both #460 and #419 so needs to take precedence. It's not quite done yet, though. Will hopefully have a chance to finish this later today.