BUG: Current way of caching is unsafe

[ksagiyam]

Describe the bug

It seems that we assume that hashing is perfect in some places. Specifically, here, we generate a sequence of 32 letters+numbers using md5 and directly use that as the disk/memory cache key:

firedrake/pyop2/caching.py

Line 477 in f473388

key = _as_hexdigest(*k), func.__qualname__

, but this is unsafe; we could pull the wrong compiled kernel in case of hash collision.

[connorjward]

I think that we have always assumed that hash collisions are so rare as to effectively never happen, and hence that this is safe "enough". For our disk caching we have to generate a hash because we need a unique file name.

[ksagiyam]

The behaviour can not be stochastic.

For disk caching, we could for instance mimic what the standard hash table does by also storing the pickled representors:

.cache/pyop2/qw/ertyuiopasdfghjklzxcvbnm123456/0/a.pickle
.cache/pyop2/qw/ertyuiopasdfghjklzxcvbnm123456/0/a.so
.cache/pyop2/qw/ertyuiopasdfghjklzxcvbnm123456/1/a.pickle
.cache/pyop2/qw/ertyuiopasdfghjklzxcvbnm123456/1/a.so

with which, upon hash-matching, we check equality by comparing the pickled values.

[connorjward]

The behaviour can not be stochastic.

For disk caching, we could for instance mimic what the standard hash table does by also storing the pickled representors:

.cache/pyop2/qw/ertyuiopasdfghjklzxcvbnm123456/0/a.pickle .cache/pyop2/qw/ertyuiopasdfghjklzxcvbnm123456/0/a.so .cache/pyop2/qw/ertyuiopasdfghjklzxcvbnm123456/1/a.pickle .cache/pyop2/qw/ertyuiopasdfghjklzxcvbnm123456/1/a.so

with which, upon hash-matching, we check equality by comparing the pickled values.

Yeah this would work. Alternatively we could just choose a "sufficiently" strong hashing algorithm like SHA-256: https://stackoverflow.com/questions/4014090/is-it-safe-to-ignore-the-possibility-of-sha-collisions-in-practice