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 iterator over isomorphic sublattices #21635

Closed
jm58660 mannequin opened this issue Oct 4, 2016 · 15 comments
Closed

LatticePoset: Add iterator over isomorphic sublattices #21635

jm58660 mannequin opened this issue Oct 4, 2016 · 15 comments

Comments

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 4, 2016

This patch will add a function to iterate over isomorphic sublattices; compare to isomorphic_subposets_iterator().

Implementation is trivial. Better ideas are welcome.

CC: @fchapoton

Component: combinatorics

Author: Jori Mäntysalo

Branch/Commit: 05efd9a

Reviewer: Travis Scrimshaw

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

@jm58660 jm58660 mannequin added this to the sage-7.4 milestone Oct 4, 2016
@jm58660 jm58660 mannequin added c: combinatorics labels Oct 4, 2016
@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 4, 2016

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 4, 2016

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

2765937Crosslinks.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 4, 2016

Commit: 2765937

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 4, 2016

New commits:

1d4c4f1Add isomorphic_sublattices_iterator().
2765937Crosslinks.

New commits:

1d4c4f1Add isomorphic_sublattices_iterator().
2765937Crosslinks.

@jm58660 jm58660 mannequin added the s: needs review label Oct 4, 2016
@kevindilks

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Oct 7, 2016

comment:5

What happens when this is given a poset which is not a lattice? Should it fail? From looking at the code, it seems like it would start going through and then sublattice would be the point of failure. Also, this makes sense for a general poset, so could we move it up to there?

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 7, 2016

comment:6

This (indirectly) calls self.meet() and self.join(), hence does not make sense to move function to posets.

In this implementation other can really be any poset, but if it is not a lattice by structure, the function will return empty list (i.e. instantly raise StopIteration). How should this behave?

@tscrim
Copy link
Collaborator

tscrim commented Oct 7, 2016

comment:7

Replying to @jm58660:

This (indirectly) calls self.meet() and self.join(), hence does not make sense to move function to posets.

Ah, yes. I agree.

In this implementation other can really be any poset, but if it is not a lattice by structure, the function will return empty list (i.e. instantly raise StopIteration). How should this behave?

You should add an example showing this happens and perhaps change the input to say - ``other`` -- finite poset.

One other little detail, remove the period in the .. SEEALSO:: blocks.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2016

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

05efd9aCheck for being lattice, not any poset.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2016

Changed commit from 2765937 to 05efd9a

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 7, 2016

comment:9

I thinked slightly more about this. We can always loose restrictions from a lattice to a poset, and so this code wants exactly lattices. It may also help if I try to invent some better way to list isomorphic sublattices.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Nov 11, 2016

comment:10

Just pinging. Is this solution (requiring other to be a lattice) OK?

@tscrim
Copy link
Collaborator

tscrim commented Nov 11, 2016

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Nov 11, 2016

comment:11

This is good with me.

@vbraun
Copy link
Member

vbraun commented Nov 12, 2016

Changed branch from u/jmantysalo/isom-sublattices-iterator to 05efd9a

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

2 participants