-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
feat(stdlib)!: Seeded hashing #2170
base: main
Are you sure you want to change the base?
feat(stdlib)!: Seeded hashing #2170
Conversation
33c4306
to
be7d875
Compare
I applied that change. Another small question on this pr, should we fix the seed used for |
We don't guarantee order currently and we don't want to make a change that makes people think the order is guaranteed, unless we decide we want to guarantee order. Either way, that's not a decision for this PR. |
This pr refactors the hashing function so we do not keep global state, it also implements a new
Hash.makeSeeded
andHash.make
function which returns aHashInstance
. If we want a different api I am happy to change things around.API:
Questions:
Hash.make
every time or do we want to use the globalSeed? Maybe if we want a globalSeed we should provide it amakeGlobal
.makeSeeded
with a set seed onMap
andSet
so we can get rid of the possible exception.My original implementation had the signature of
Hash.make: (seed: Number) => (anything: a) => Number
but this causes our inference engine to lock the hashing function a particular data type when you do something like:The alternative approach suggested was to use a submodule for seeded but as the api's would differ this did not seem ideal.
One benefit to this approach is because we call
Random.random()
onmake
we will not throw a possible exception onHash.hash
which means all of ourSet
andMap
functions besidesmake
wont throw.Succeeds: #1952
This is breaking as we changed the type of
Hash.hash
from(a) => Number
to(HashInstance, a) => Number
.