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

LatticePoset: add maximal_sublattices() #18567

Closed
jm58660 mannequin opened this issue Jun 1, 2015 · 32 comments
Closed

LatticePoset: add maximal_sublattices() #18567

jm58660 mannequin opened this issue Jun 1, 2015 · 32 comments

Comments

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Jun 1, 2015

This adds a new function that computes maximal (proper) sublattices of the lattice.

It could be faster to reverse thinking: instead of computing sublattice try to get a minimal "remainder" of sublattice. For example the remainder must be a connected poset and it's every minimal element may not have more than one lower cover. However, this slow implementation at least works, and can be used as a checkpoint when making faster implementations.

CC: @nathanncohen @kevindilks

Component: combinatorics

Author: Jori Mäntysalo, Travis Scrimshaw

Branch/Commit: d85c873

Reviewer: Travis Scrimshaw, Jori Mäntysalo

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

@jm58660 jm58660 mannequin added c: combinatorics labels Jun 1, 2015
@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jun 10, 2015

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jun 10, 2015

Commit: 4b47df9

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jun 10, 2015

comment:2

There is now some code that at least seems to work. This is quite interesting optimization problem. Can this be made to real maximal_sublattices_iterator() instead of maximal_sublattices() (that internally generates some non-maximal sublattices and then discards them).

I tried googling some time, but find no algorithm for this. This is somewhat ad hoc solution.


New commits:

4b47df9Added a function.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 12, 2015

Changed commit from 4b47df9 to 5ede78b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 12, 2015

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

5ede78bSome optimization.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 8, 2015

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ef073c1New function for maximal_sublattices.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 8, 2015

Changed commit from 5ede78b to ef073c1

@jm58660

This comment has been minimized.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jul 8, 2015

Author: Jori Mäntysalo

@jm58660 jm58660 mannequin changed the title LatticePoset: add maximal_sublattices_iterator() LatticePoset: add maximal_sublattices() Jul 8, 2015
@jm58660 jm58660 mannequin added the s: needs review label Jul 8, 2015
@jm58660 jm58660 mannequin added this to the sage-6.8 milestone Jul 8, 2015
@jm58660 jm58660 mannequin removed the wishlist item label Jul 8, 2015
@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jul 19, 2015

comment:6

Ping... Anybody wants to look at the code? This is necessary part in the road to frattini_sublattice().

@tscrim
Copy link
Collaborator

tscrim commented Jul 27, 2015

comment:8

Two minor things:

  • I think you could change the test sorted([sorted(list(x)) for x in ms]) to sorted(ms, key=sorted) (note that I didn't actually try this).

  • Could you add a space around the < comparisons to help improve readability?

Kevin, if you want to do the full review, please go ahead. I'm not sure I will have enough time to throughly test this.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2015

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

8dbb1d8Two minor changes.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2015

Changed commit from ef073c1 to 8dbb1d8

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jul 27, 2015

comment:10

Here is one test lattice:

z={'d': [1,2,3], 1:['l', 4], 2:[4,'c'], 3:['r',4], 4:[5], 5:[6,13], 6:[7,11], 7:[8,14], 8:[9,12], 9:[10,15], 10:['u'], 'l':[11], 11:[12], 12:['u'], 'r':[13], 13:[14], 14:[15], 15:['u'], 'c':[10]}
M3min=LatticePoset(z)

(from http://www.math.hawaii.edu/~ralph/Preprints/combined.pdf you can find more). Do you want some test code? You can make a random lattice (for example random poset + completion_by_cuts()), and test by adding one random element to maximal sublattice - it should generate whole lattice. Or make a test like #18562 and see if a random maximal sublattice is found in the list of sublattices.

@jm58660 jm58660 mannequin modified the milestones: sage-6.8, sage-6.9 Jul 27, 2015
@fchapoton
Copy link
Contributor

comment:12

needs rebase, does not apply

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2015

Changed commit from 8dbb1d8 to 24b10dd

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2015

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

24b10ddAdd maximal_sublattices().

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Aug 9, 2015

comment:14

Replying to @fchapoton:

needs rebase, does not apply

Done that, should work now. I will change this to needs_review after checking myself that this compiles and works.

@jm58660 jm58660 mannequin added s: needs review and removed s: needs work labels Aug 9, 2015
@tscrim
Copy link
Collaborator

tscrim commented Aug 23, 2015

@tscrim
Copy link
Collaborator

tscrim commented Aug 23, 2015

Changed commit from 24b10dd to none

@tscrim
Copy link
Collaborator

tscrim commented Aug 23, 2015

comment:16

I made some minor reviewer changes. If you're happy with them, then go ahead and set a positive review.

@tscrim
Copy link
Collaborator

tscrim commented Aug 23, 2015

Reviewer: Travis Scrimshaw

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Aug 24, 2015

comment:17

?? Branch-field is red, so I guess you made a typo.

@tscrim
Copy link
Collaborator

tscrim commented Aug 24, 2015

comment:18

I think it's because beta3 was just released and my branch is based off beta2.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Aug 24, 2015

comment:19

Replying to @tscrim:

I think it's because beta3 was just released and my branch is based off beta2.

But I don't see the "commits"-link, like there is when the code needs rebasing.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 24, 2015

Commit: d85c873

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 24, 2015

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

24b10ddAdd maximal_sublattices().
ffe4fb8Merge branch 'u/jmantysalo/latticeposet__add_maximal_sublattices_iterator__' of git://trac.sagemath.org/sage into u/jmantysalo/latticeposet__add_maximal_sublattices_iterator__
d85c873Some reviewer tweaks.

@tscrim
Copy link
Collaborator

tscrim commented Aug 24, 2015

comment:21

It seems like it didn't push it for some reason. Fixed.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Aug 25, 2015

Changed author from Jori Mäntysalo to Jori Mäntysalo, Travis Scrimshaw

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Aug 25, 2015

comment:22

Replying to @tscrim:

I made some minor reviewer changes. If you're happy with them, then go ahead and set a positive review.

Thank you! Compiles & works.

(Now we just put this to production, and wait for someone to come up with better algorithm...)

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Aug 25, 2015

Changed reviewer from Travis Scrimshaw to Travis Scrimshaw, Jori Mäntysalo

@vbraun
Copy link
Member

vbraun commented Aug 26, 2015

Changed branch from u/tscrim/maximal_sublattices_iterator-18567 to d85c873

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