Sage: Ticket #19331: Make Element.__hash__ return 0 by default
https://trac.sagemath.org/ticket/19331
<p>
In <a class="closed ticket" href="https://trac.sagemath.org/ticket/19321" title="enhancement: provide better hash functions (closed: fixed)">#19321</a> a stopgap is implemented in Element that would end up with a lot of annoying messages. We propose another solution (actually we follow the initial proposition from <a class="closed ticket" href="https://trac.sagemath.org/ticket/19016" title="defect: Better hash for Element (closed: fixed)">#19016</a>): return 0 as a default hash.
</p>
<p>
This ticket is just an alternative to <a class="closed ticket" href="https://trac.sagemath.org/ticket/19321" title="enhancement: provide better hash functions (closed: fixed)">#19321</a>.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/19331
Trac 1.1.6vdelecroixFri, 02 Oct 2015 00:21:43 GMTstatus changed; branch set
https://trac.sagemath.org/ticket/19331#comment:1
https://trac.sagemath.org/ticket/19331#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>branch</strong>
set to <em>u/vdelecroix/19331</em>
</li>
</ul>
TicketgitFri, 02 Oct 2015 00:21:56 GMTcommit set
https://trac.sagemath.org/ticket/19331#comment:2
https://trac.sagemath.org/ticket/19331#comment:2
<ul>
<li><strong>commit</strong>
set to <em>64053a2645456e605061a24952568e3c5e1f51e2</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=64053a2645456e605061a24952568e3c5e1f51e2"><span class="icon"></span>64053a2</a></td><td><code>Trac 19331: return 0 as a default hash for Element</code>
</td></tr></table>
TicketvdelecroixFri, 02 Oct 2015 14:28:40 GMT
https://trac.sagemath.org/ticket/19331#comment:3
https://trac.sagemath.org/ticket/19331#comment:3
<p>
The only problem seems to be about the output order... only a tiny bit of work needed to make it efficient.
</p>
TicketnbruinFri, 02 Oct 2015 15:57:40 GMT
https://trac.sagemath.org/ticket/19331#comment:4
https://trac.sagemath.org/ticket/19331#comment:4
<p>
Changing the runtime cost of data structures from what it should be should not be taken lightly. I don't think this change leads to acceptable behaviour (and it will be harder to find what's wrong: a hashing error is easier to debug)
</p>
<pre class="wiki">class A(object):
def __init__(self,N):
self.N=N
def __hash__(self):
return 0
def __eq__(self,other):
return isinstance(other,A) and self.N==other.N
def __call__(self):
return self.N
class B(object):
def __init__(self,N):
self.N=N
def __hash__(self):
return hash(self.N)
def __eq__(self,other):
return isinstance(other,B) and self.N==other.N
def __call__(self):
return self.N
@cached_function
def T(a):
return a()
</pre><pre class="wiki">sage: %timeit( [T(B(n)) for n in range(10000)])
100 loops, best of 3: 14.3 ms per loop
sage: %timeit( [T(A(n)) for n in range(10000)])
1 loops, best of 3: 25.8 s per loop
</pre><p>
Or, with a smaller cache it's still bad (I've cleaned the cache):
</p>
<pre class="wiki">sage: %timeit( [T(A(n)) for n in range(100)])
100 loops, best of 3: 2.22 ms per loop
sage: %timeit( [T(B(n)) for n in range(100)])
10000 loops, best of 3: 181 µs per loop
</pre><p>
Even for a dictionary of size 10, the trivial hash still causes a 4x slowdown.
</p>
<p>
A change like this can push code that runs correctly in sage at present to a point where running it is not practical. I seriously think there may be a user out there that has an application that depends on this code. This change would break sage for him in that respect. I am strongly -1 to introducing something like that in a release candidate, just before release, because we wouldn't catch it.
</p>
TicketjdemeyerThu, 08 Oct 2015 16:15:12 GMTstatus changed
https://trac.sagemath.org/ticket/19331#comment:5
https://trac.sagemath.org/ticket/19331#comment:5
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Many doctest failures...
</p>
TicketvbraunSat, 10 Oct 2015 13:14:31 GMTmilestone changed
https://trac.sagemath.org/ticket/19331#comment:6
https://trac.sagemath.org/ticket/19331#comment:6
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.9</em> to <em>sage-6.10</em>
</li>
</ul>
TicketnbruinWed, 28 Oct 2015 15:20:49 GMTstatus, milestone changed
https://trac.sagemath.org/ticket/19331#comment:7
https://trac.sagemath.org/ticket/19331#comment:7
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-6.10</em> to <em>sage-duplicate/invalid/wontfix</em>
</li>
</ul>
<p>
with <a class="closed ticket" href="https://trac.sagemath.org/ticket/19016" title="defect: Better hash for Element (closed: fixed)">#19016</a> merged we don't need this ticket anymore.
</p>
TicketvdelecroixWed, 28 Oct 2015 20:52:22 GMTstatus changed; reviewer set; commit, branch, author deleted
https://trac.sagemath.org/ticket/19331#comment:8
https://trac.sagemath.org/ticket/19331#comment:8
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>commit</strong>
<em>64053a2645456e605061a24952568e3c5e1f51e2</em> deleted
</li>
<li><strong>reviewer</strong>
set to <em>Vincent Delecroix</em>
</li>
<li><strong>branch</strong>
<em>u/vdelecroix/19331</em> deleted
</li>
<li><strong>author</strong>
<em>Vincent Delecroix</em> deleted
</li>
</ul>
<p>
Right!
</p>
TicketvbraunThu, 29 Oct 2015 08:53:09 GMTstatus changed; resolution set
https://trac.sagemath.org/ticket/19331#comment:9
https://trac.sagemath.org/ticket/19331#comment:9
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
Ticket