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

PlanePartitions are not hashable #36118

Closed
2 tasks done
mantepse opened this issue Aug 22, 2023 · 4 comments · Fixed by #36124
Closed
2 tasks done

PlanePartitions are not hashable #36118

mantepse opened this issue Aug 22, 2023 · 4 comments · Fixed by #36124

Comments

@mantepse
Copy link
Collaborator

Steps To Reproduce

sage: p = PlanePartitions(4).an_element(); p
Plane partition [[4]]
sage: hash(p)
Traceback (most recent call last):
...
TypeError: unhashable type: 'list'

Expected Behavior

sage: p = PlanePartitions(4).an_element()
sage: hash(p)

should return a hash.

Actual Behavior

A TypeError is raised.

Additional Information

Plane partitions inherit from ClonableArray, but the elements of the array are lists, eg.:

    def __init__(self, parent, pp, check=True):
...
        if isinstance(pp, PlanePartition):
            ClonableArray.__init__(self, parent, pp, check=False)
        else:
            pp = [list(_) for _ in pp]
            if pp:
                for i in reversed(range(len(pp))):
                    while pp[i] and not pp[i][-1]:
                        del pp[i][-1]
                    if not pp[i]:
                        pp.pop(i)
            ClonableArray.__init__(self, parent, pp, check=check)
...

Environment

irrelevant.

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@Cobord
Copy link

Cobord commented Aug 23, 2023

Where are some places where PlanePartitions are being put into a set? Or any other places something that expects to be able to hash PlanePartitions but cannot?

@mantepse
Copy link
Collaborator Author

This is mostly needed in user code. For example, imagine that you want to find a bijection between TSSCPP and ASM:

sage: n = 4
sage: A = AlternatingSignMatrices(n)
sage: B = PlanePartitions([2*n]*3, symmetry='TSSCPP')
sage: bij = Bijectionist(A, B)
# next, you would specify some statistics or maps to be preserved
# ...
sage: ascii_art(list(bij.minimal_subdistributions_iterator()))

@Cobord
Copy link

Cobord commented Aug 25, 2023

Then I wouldn't say a bug because that is a more general issue. This is described well with the distinction of frozenset and set. That idea works around something mutating under your feet with a version that can't. For this example, what if there was a method that does some time step of a box falling problem that mutates the partition. I don't want to store every step as a brand new immutable object when I just want the final partition for some calculation about limit shapes.

@mantepse
Copy link
Collaborator Author

This sounds an awful lot like premature optimization. Why are you afraid of creating a new plane partition? Note that, in general, if you add a single box to an existing one, you may be copying the old one anyway. If you really need to squeeze out all you can I think it would be best to create a specialized class for "chains" of plane partitions.

vbraun pushed a commit to vbraun/sage that referenced this issue Aug 27, 2023
…gemath#36116

    
* Fix a thinko in the iterator for TSSCPP
* make plane partitions hashable
* avoid rational numbers in the cardinality methods, when necessary
* raise an error when an integer and a symmetry is provided as argument
to `PlanePartitions`

Fixes sagemath#36119
Fixes sagemath#36118
Fixes sagemath#36116

- [X] The title is concise, informative, and self-explanatory.
- [X] The description explains in detail what this PR is about.
- [X] I have linked a relevant issue or discussion.
- [X] I have created tests covering the changes.
    
URL: sagemath#36124
Reported by: Martin Rubey
Reviewer(s): Frédéric Chapoton
@mkoeppe mkoeppe added this to the sage-10.2 milestone Aug 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants