Context Navigation

← Previous Ticket
Next Ticket →

#14763 closed enhancement (fixed)

Suter's diagonal slides on subsets of the partition lattice

Reported by:	darij	Owned by:	tbd
Priority:	major	Milestone:	sage-5.11
Component:	combinatorics	Keywords:	days49, partitions
Cc:	sage-combinat, tscrim,	Merged in:	sage-5.11.beta3
Authors:	Darij Grinberg	Reviewers:	Travis Scrimshaw
Report Upstream:	N/A	Work issues:
Branch:		Commit:
Dependencies:	#8392	Stopgaps:

Description (last modified by tscrim)

This patch implements Suter's dihedral group action on (subsets of) the partition lattice, and fixes some typos in docstrings.

Apply:

Attachments (3)

trac_14763-suter_slides-dg.patch (7.8 KB) - added by darij 8 years ago.: Updated version, now also computes the inverse of the Suter slide without iterating it n-1 times
trac_14763-rereview-dg.patch (4.3 KB) - added by darij 8 years ago.: minor changes to Travis's review
trac_14763-review-ts.patch (10.8 KB) - added by tscrim 8 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 8 years ago by darij

Description modified (diff)
Status changed from new to needs_review

comment:2 Changed 8 years ago by darij

Component changed from PLEASE CHANGE to combinatorics

Changed 8 years ago by darij

Attachment trac_14763-suter_slides-dg.patch added

Updated version, now also computes the inverse of the Suter slide without iterating it n-1 times

comment:3 Changed 8 years ago by tscrim

Description modified (diff)
Reviewers set to Travis Scrimshaw

Review patch which removes non-ascii characters, reworks the doc, and checks that the input is valid. If you're happy with my changes, go ahead and set this to positive reivew.

Changed 8 years ago by darij

Attachment trac_14763-rereview-dg.patch added

minor changes to Travis's review

comment:4 Changed 8 years ago by darij

I agree with most of your review; thanks for catching the non-ascii copypasta in particular. I've made checking optional and reinstated the "length = m" condition in the docstring since various people (like me) consider (2, 1), (2, 1, 0) and (2, 1, 0, 0, 0, ...) to be three equivalent ways to write one and the same partition.

comment:5 Changed 8 years ago by darij

Description modified (diff)

comment:6 Changed 8 years ago by tscrim

Description modified (diff)

Here's the new review patch. Come find me if you want anything else changed.

Apply: trac_14763-suter_slides-dg.patch trac_14763-review-ts.patch

comment:7 Changed 8 years ago by darij

Please put the exp vs. n/2 check back.

Lines 1781-1784 are redundant. That's my bad, sorry. I thought if the empty partition case was necessary in the positive exp case, it would also be in the negative exp one; but it isn't.

Lines 1785-1786 have a (now redundant) check.

Thanks for reviewing!

comment:8 Changed 8 years ago by tscrim

The empty partition case is necessary in both positive and negative exp; there's even a doctest on it on line 1696.

Apply: trac_14763-suter_slides-dg.patch trac_14763-review-ts.patch

comment:9 Changed 8 years ago by darij

I'm fine with it either way; I just don't see how anything will change if lines 1783-1786 are removed. Not like checking an int for being 0 is a time issue...

comment:10 Changed 8 years ago by tscrim

So positive review?

comment:11 Changed 8 years ago by darij

Status changed from needs_review to positive_review

Yes.

comment:12 Changed 8 years ago by darij

Oh, here's something I forgot to write. Line 1781 could (and I think should) be a simple "else", since we're in the while-loop. If you agree, Travis, I'll change it.