-
Notifications
You must be signed in to change notification settings - Fork 162
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
Implement manhattan and chebyshev distance for integer vectors #593
Conversation
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.
Thanks for the PR.
abs_diff
on signed integers returns an unsigned integer so this will need a bit more work to code generate that .
The original #415 PR returned the result in a larger sized integer which for the Manhattan distance would help avoid overflow, however I guess for higher dimensions than 2 overflow would still be possible, so I'm not sure it's worth doing that. Also while glam doesn't currently have a I128Vec*
or U128Vec*
if they're ever added there is no u256
primitive in Rust which would make returning a larger integer in with any 128 sizes integer vector not possible. Give that I'm leaning towards your implementation and not returning a larger integer type, but adding a comment to the method pointing out that the result could overflow so that the caller is aware. Do you have any thoughts on how overflow should be handled for these?
Could you also add some tests for these new methods as part of this PR.
We should use unsigned integer as return type. As per the documentation of
For |
Thanks! |
This implements #415 using the code generator.