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

The hash function of combinatorial free module elements should depend on the parent #15959

Open
nthiery opened this issue Mar 17, 2014 · 15 comments

Comments

@nthiery
Copy link
Contributor

nthiery commented Mar 17, 2014

It would be desirable to make the hash value of (combinatorial) free
module elements depend on the hash value of the parent. However this
currently breaks some code (see
e.g. sage.combinat.crystals.kyoto_path_model.KyotoPathModel).

This code indeed assumes, in dictionary look-ups, that an element of
some free module over ZZ and its counter part over QQ (in the case at
hand, the weight lattice and the weight space), comparing as equal
because of a coercion, have the same hash value.

Yet another case of equality using coercion being harmful:
http://wiki.sagemath.org/EqualityCoercion

Depends on #15931
Depends on #16193

CC: @tscrim @anneschilling

Component: linear algebra

Author: Travis Scrimshaw

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

@nthiery nthiery added this to the sage-6.2 milestone Mar 17, 2014
@tscrim
Copy link
Collaborator

tscrim commented Mar 17, 2014

comment:1

Nicolas, do you have a preliminary branch that you could post (or is this the #15931 branch?) or could post the errors/doctest failures?

@nthiery
Copy link
Contributor Author

nthiery commented Mar 17, 2014

comment:3

Replying to @tscrim:

Nicolas, do you have a preliminary branch that you could post (or is this the #15931 branch?) or could post the errors/doctest failures?

Sorry, I should have provided steps to reproduce. Checkout branch #15931 at commit a2bbe7f, and run the tests on the Kyoto path model.

@tscrim
Copy link
Collaborator

tscrim commented Mar 28, 2014

Author: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Mar 28, 2014

Commit: 7bdb017

@tscrim
Copy link
Collaborator

tscrim commented Mar 28, 2014

Branch: u/tscrim/kyoto_fix-15959

@tscrim
Copy link
Collaborator

tscrim commented Mar 28, 2014

Dependencies: #15931

@tscrim
Copy link
Collaborator

tscrim commented Mar 28, 2014

comment:4

I haven't merged in the latest #15931, but instead started with the a2bbe7f commit. The fix for the Kyoto path model is that I was passing in elements of the weight space, whereas the Epsilon()/Phi() methods returned an element of the weight lattice. I've added an additional check for robustness.


New commits:

e8fe5ebTrac 15931: implement a proper hash function for (combinatorial) free module elements
e6ef605Trac 15931: add a @cached_method on the new hash function for (combinatorial) free module elements
a2bbe7fTrac 15931: include the hash of the parent in the new hash function for (combinatorial) free module elements
13be620Merge branch 'develop' into test/15959
7bdb017Fix for Kyoto path model with hash depending on the parent.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 9, 2014

Changed commit from 7bdb017 to bbf4917

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 9, 2014

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

2c8ba97Merge branch 'develop' into u/tscrim/kyoto_fix-15959
bbf4917Merge branch 'develop' into u/tscrim/kyoto_fix-15959

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 15, 2014

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

e778bafMerge branch 'develop' into u/tscrim/kyoto_fix-15959

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 15, 2014

Changed commit from bbf4917 to e778baf

@tscrim
Copy link
Collaborator

tscrim commented Apr 20, 2014

comment:7

I've merged the branch into #16193 and set that as a dependency.

@tscrim
Copy link
Collaborator

tscrim commented Apr 20, 2014

Changed dependencies from #15931 to #15931 #16193

@tscrim
Copy link
Collaborator

tscrim commented Apr 20, 2014

Changed commit from e778baf to none

@tscrim
Copy link
Collaborator

tscrim commented Apr 20, 2014

Changed branch from u/tscrim/kyoto_fix-15959 to none

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

3 participants