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

do not include parent in hash of parking functions #34824

Closed
mantepse opened this issue Dec 5, 2022 · 18 comments
Closed

do not include parent in hash of parking functions #34824

mantepse opened this issue Dec 5, 2022 · 18 comments

Comments

@mantepse
Copy link
Collaborator

mantepse commented Dec 5, 2022

We do not want the following:

sage: P = ParkingFunctions(3)
sage: f = P([1,1,1])
sage: g = ParkingFunction([1,1,1])
sage: f in P
True
sage: g in P
True
sage: f in list(P)
True
sage: g in list(P)
True
sage: f in set(P)
True
sage: g in set(P)
False

CC: @nthiery

Component: combinatorics

Author: Martin Rubey

Branch/Commit: 49ff03d

Reviewer: Travis Scrimshaw

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

@mantepse mantepse added this to the sage-9.8 milestone Dec 5, 2022
@mantepse
Copy link
Collaborator Author

mantepse commented Dec 5, 2022

@mantepse
Copy link
Collaborator Author

mantepse commented Dec 5, 2022

Commit: ec0577e

@mantepse
Copy link
Collaborator Author

mantepse commented Dec 5, 2022

New commits:

ec0577edo not inherit hash from list_clone.pyx

@mantepse

This comment has been minimized.

@mantepse
Copy link
Collaborator Author

mantepse commented Dec 5, 2022

Author: Martin Rubey

@nbruin
Copy link
Contributor

nbruin commented Dec 5, 2022

comment:3

I expect that you'll have to redefine equality comparison as well: __hash__ and __eq__ go hand-in-hand. See

https://docs.python.org/3/reference/datamodel.html#object.__hash__

You could try to override _hash_ instead, which is the internal routine that is used by the default __hash__ to populate the _hash_ cache attribute.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 5, 2022

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

6e8ed10override `_hash_` instead of __hash__

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 5, 2022

Changed commit from ec0577e to 6e8ed10

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2022

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

e8abff3do not include the parent in the hash of CloneableArray and ClonableIntArray, and remove useless overrides
32a2bb4undo override

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2022

Changed commit from 6e8ed10 to 32a2bb4

@mantepse
Copy link
Collaborator Author

comment:7

I do not understand this - shouldn't the hash be inherited from ClonableArray?

Is this because of metaclass=InheritComparisonClasscallMetaclass?

sage: A = SetPartition([[1],[2,3],[4]])
sage: hash(A)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In [3], line 1
----> 1 hash(A)

TypeError: unhashable type: 'SetPartitions_all_with_category.element_class'

@mantepse
Copy link
Collaborator Author

comment:8

No, for ParkingFunction, hashing works, even though

class ParkingFunction(ClonableArray, metaclass=InheritComparisonClasscallMetaclass):

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2022

Changed commit from 32a2bb4 to 49ff03d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2022

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

49ff03dcheck that hash and equality works in DecoratedPermutation, restore hash in OrderedMultisetPartitionIntoSets and AbstractSetPartition

@mantepse
Copy link
Collaborator Author

comment:10

See also https://groups.google.com/g/sage-devel/c/0J2jiBh5AvE

@tscrim
Copy link
Collaborator

tscrim commented Dec 14, 2022

comment:11

LGTM. This will make handling equality easier for subclasses.

Note that this will cause a slight performance regression when there is hashing involving things that are represented by the same list but should not be compared. For example [3, 2, 1] as a permutation versus as a partition (if they both inherited from ClonableArray). However, I don't see this having a practical effect and it is still outweighed by the ease-of-use IMO.

@tscrim
Copy link
Collaborator

tscrim commented Dec 14, 2022

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Jan 5, 2023

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