JuliaPy / SymPy.jl

Julia interface to SymPy via PyCall
http://juliapy.github.io/SymPy.jl/
MIT License
266 stars 61 forks source link

Overload for `hash(::Sym, ::UInt64)` missing #520

Closed jlbosse closed 10 months ago

jlbosse commented 10 months ago

Currently the implementation of hash(x::Sym, h::UInt64) depends on the object id and not value of x, whereas hash(x::Sym) depends on the value of x. This leads to counter intuitive behaviour like the following:

using SymPy

t = symbols("t")
s = sin(t)

hash(s) == hash(sin(t))
# true

hash((sin(t),)) == hash((s,))
# false

# because 
hash(sin(t), zero(UInt64)) == hash(s, zero(UInt64))
# false

# even though
(sin(t),) == (s,)
# true

See also this issue for consequences of the missing hash function and this discussion on discourse for why this is an issue.

I think, and the julia documentation also clearly states, that the hash function should satisfy x == y implies hash(x) == hash(y).

I am happy to create a PR that implements the two-argument version hash(x::Sym, h::UInt64) which should fix this; If that is the desired behaviour.

jverzani commented 10 months ago

Thanks for this. Following your discourse discussion, I put in a PR to PyCall (https://github.com/JuliaPy/PyCall.jl/pull/1054) which along with the definition hash(x::Sym, h::UInt64) = hash(PyObject(x), h) should get this done. Let me see if that is accepted and if not, look to solve this within SymPy.