-
Notifications
You must be signed in to change notification settings - Fork 37
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
Use MurmurHash3 to hash the algorithm name for the algorithm type in ParticleIDMeta #307
Conversation
Copying the murmurhash files every time we want to hash something isn't great... |
I agree, but I am not sure where to put them for EDM4hep, and I don't really want to expose them from podio. |
A possibility could be to make a new Murmurhash package and build that library. Can any other hashes be considered, like There is already a spack package for Murmurhash, but the main purpose seems to be its python bindings, we would probably need some CMake changes: https://github.com/spack/spack/blob/develop/var/spack/repos/builtin/packages/py-murmurhash/package.py to make it findable |
I don't have a strict preference for any hashing algorithm. In the end the main requirement is that it has to fit into 32 bits. Whether we want to pull in boost just for that is another question though. |
Unless there are any more comments, I would merge this sometime tomorrow (synchronously with the necessary API changes in key4hep/k4EDM4hep2LcioConv#68) |
BEGINRELEASENOTES
algoType
in theedm4hep::utils::ParticleIDMeta
use the 32 bit version of MurmurHash3 to hash thealgoName
to get toalgoType
. Fixes Stable / consistent algorithm types for PIDHandler #298algoType
a private member but allow read access to it via thealgoType
member function.algoType
has to be filled on construction. It is still possible to set it manually, the hashing will only kick in for the constructor taking only a name.ENDRELEASENOTES
We could consider making
ParticleID::algoType
auint32_t
field for this, as that would make it a bit more easily visible that this is potentially a hash.