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

hashCode for Vector3 etc do not work well #198

Closed
robertmuth opened this issue Dec 31, 2018 · 9 comments
Closed

hashCode for Vector3 etc do not work well #198

robertmuth opened this issue Dec 31, 2018 · 9 comments

Comments

@robertmuth
Copy link

If two vectors are close to zero they will map to "0" in the current hashCode implementation.
A better approach would be something like this:

int hashVector3(VM.Vector3 v) {
int h = 0;
for (int i in v.storage.buffer.asInt32List()) {
h ^= i;
}
return h;
}

@johnmccutchan
Copy link
Collaborator

Please provide a pull request with unit tests.

@rakudrama
Copy link
Collaborator

Is this fixed by the fix for dart-lang/sdk#35116?

@robertmuth
Copy link
Author

@john
I am not the right person to create the pull request. It will probably take me hours to familiarize
myself with the code base and test framework, while it should take only minutes for somebody
more familiar with the project. Also, I do not plan on submitting a lot of pull requests in the future
so the learning effort would never pay dividends.

@rakudrama
Copy link
Collaborator

@robertmuth There have been changes in the hashCode of numbers. Can you verify that this is still a problem?

@robertmuth
Copy link
Author

I just tracked this down for Vector3. We have

class Vector3 implements Vector {
final Float32List _v3storage;
...

@OverRide
int get hashCode => quiver.hashObjects(_v3storage);
...
}

elsewhere we have

import 'hash.dart' as quiver;

where hash.dart is this file:
https://github.com/google/vector_math.dart/blob/master/lib/hash.dart

The code for hashObjects is a functional programmers dream, incomprehensible,
and probably wrong.
If I understand the code correctly, we are still passing in a List and
each double gets implicitly converted to int.

@rakudrama
Copy link
Collaborator

@robertmuth
The code was changed in January, and I commented to that effect above in April.
The intention of the change is that doubles are not implicitly converted to int while maintaining that equal values (1 and 1.0) have the same hash code.
So does the hashCode for Vector3 still not work well?
Presumably your initial report came from some experience that you could revisit.

@robertmuth
Copy link
Author

The sad part is that I do not remember where I encountered it anymore.

But if hash of Vector3(0.1, 0.1, 0.1) != hash of Vector3(0.2, 0.2, 0.2)
the problem has been fixed and I stay corrected.

BTW: I see now the "i.hashCode" part in hashObjects so yeah there is no
double to int conversion - my mistake.

@robertmuth
Copy link
Author

I did some spot checking and the problem is gone.

@rakudrama
Copy link
Collaborator

Thanks!

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

No branches or pull requests

3 participants