Skip to content
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

Hash broken for unions, intersections, differences, and symmetric differences of sets #14432

Closed
saraedum opened this issue Apr 9, 2013 · 20 comments

Comments

@saraedum
Copy link
Member

saraedum commented Apr 9, 2013

The following code raises an exception

sage: S=Set(ZZ).union(Set([infinity]))
sage: hash(S)
Traceback (most recent call last):
...
RuntimeError: maximum recursion depth exceeded

The same should happen for intersections, differences, and symmetric differences.

Component: combinatorics

Keywords: hash, set, sd59

Author: Julian Rueth

Branch/Commit: 05ed037

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/14432

@saraedum
Copy link
Member Author

saraedum commented Apr 9, 2013

Attachment: trac_14432.patch.gz

@saraedum
Copy link
Member Author

saraedum commented Apr 9, 2013

Author: Julian Rueth

@saraedum
Copy link
Member Author

saraedum commented Apr 9, 2013

comment:2

I do not have access to a 32-bit machine right now. Could someone fill in the missing hash values? Is there a way to compute them from the 64-bit values btw?

@saraedum
Copy link
Member Author

saraedum commented Apr 9, 2013

comment:3

I decided to remove some duplicated code rather than adding a __hash__ method to all the implementations - hope that is ok.

@tscrim
Copy link
Collaborator

tscrim commented Apr 9, 2013

comment:4

To test hashing, it is better to do tests like hash(s) == hash(s) that do not depend upon the specific value but instead show that the object is hashable and it's value does not change.

@saraedum
Copy link
Member Author

saraedum commented Apr 9, 2013

comment:5

That's a good point. Thanks for the input. I will change my patch accordingly.

@saraedum
Copy link
Member Author

Branch: u/saraedum/ticket/14432

@saraedum

This comment has been minimized.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 28, 2013

Commit: e851c6f

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 28, 2013

comment:9

This patch does not apply cleanly on 5.13.beta0. And it's probably because it contains many dependencies #14482 O_o

~/sage$ git fetch trac u/saraedum/ticket/14432:hash
remote: Counting objects: 17, done.
remote: Compressing objects: 100% (12/12), done.
remote: Total 12 (delta 10), reused 0 (delta 0)
Unpacking objects: 100% (12/12), done.
From ssh://trac.sagemath.org:2222/sage
 * [new branch]      u/saraedum/ticket/14432 -> hash
~/sage$ git merge hash
Auto-merging src/sage/sets/set.py
CONFLICT (content): Merge conflict in src/sage/sets/set.py
Auto-merging src/sage/doctest/forker.py
CONFLICT (content): Merge conflict in src/sage/doctest/forker.py
Auto-merging build/pkgs/sagenb/package-version.txt
CONFLICT (content): Merge conflict in build/pkgs/sagenb/package-version.txt
Auto-merging build/pkgs/sagenb/checksums.ini
CONFLICT (content): Merge conflict in build/pkgs/sagenb/checksums.ini
Auto-merging build/pkgs/sagenb/SPKG.txt
CONFLICT (content): Merge conflict in build/pkgs/sagenb/SPKG.txt
Automatic merge failed; fix conflicts and then commit the result.

Last 10 new commits:

[changeset:e851c6f]fixed doctest for the hash of a union/intersection/... of sets
[changeset:10a3d1b](HEAD, refs/heads/stefan) Trac #14432: Fix `__hash__` for unions of sets
[changeset:a10bc4f]Merge branch 'ticket/14482'
[changeset:fda55a9]Merge branch 'u/saraedum/ticket/14482' of ssh://trac.sagemath.org:2222/sage into HEAD
[changeset:61236da]Merge remote-tracking branch 'trac/public/sage-git/ticket/14482'
[changeset:08d3e94]Merge branch 'ticket/14482'
[changeset:50e1ece]Merge branch 'ticket/14482'
[changeset:1ca56cf]Merge branch 'ticket/14273' into tip
[changeset:aa60895]remove workaround for sagenb pull request 84 (#14273)
[changeset:133785b]Merge branch 'ticket/14482' into tip

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.0, sage-6.1 Dec 17, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 26, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

08f5ee0Merge branch 'develop' into ticket/14432

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 26, 2014

Changed commit from e851c6f to 08f5ee0

@saraedum
Copy link
Member Author

Changed keywords from hash, set to hash, set, sd59

@tscrim
Copy link
Collaborator

tscrim commented Jul 2, 2014

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jul 2, 2014

comment:16

I've made some minor review tweaks. If you're happy with them, then positive review.


New commits:

0190e4dSome review changes to set.py.

@tscrim
Copy link
Collaborator

tscrim commented Jul 2, 2014

Changed commit from 08f5ee0 to 0190e4d

@tscrim
Copy link
Collaborator

tscrim commented Jul 2, 2014

Changed branch from u/saraedum/ticket/14432 to public/ticket/14432

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 2, 2014

Changed commit from 0190e4d to 05ed037

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 2, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

05ed037Fix typo.

@vbraun
Copy link
Member

vbraun commented Jan 24, 2015

Changed branch from public/ticket/14432 to 05ed037

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants