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 meet_irreducibles, faster is_distributive #17121

Closed
jm58660 mannequin opened this issue Oct 9, 2014 · 17 comments
Closed

LatticePoset: Add meet_irreducibles, faster is_distributive #17121

jm58660 mannequin opened this issue Oct 9, 2014 · 17 comments

Comments

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 9, 2014

Finite lattice posets now has join_irreducibles() and join_irreducibles_poset(). Add meet_irreducibles() and meet_irreducibles_poset().

is_distributive() is now based on formal definition. It can just check that lattice is graded and that number of both meet- and join-irreducibles equals to rank (=height-1) of the lattice.

CC: @nathanncohen

Component: combinatorics

Author: Jori Mäntysalo

Branch/Commit: 8b68cdb

Reviewer: Nathann Cohen

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

@jm58660 jm58660 mannequin added this to the sage-6.4 milestone Oct 9, 2014
@jm58660 jm58660 mannequin added c: combinatorics labels Oct 9, 2014
@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 9, 2014

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 9, 2014

New commits:

0ee134cMuch better is_distributive(). Added two little functions.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 9, 2014

Commit: 0ee134c

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 9, 2014

Author: Jori Mäntysalo

@jm58660 jm58660 mannequin added the s: needs review label Oct 9, 2014
@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 10, 2014

Reviewer: Nathann Cohen

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 10, 2014

comment:3

Hello !

  1. meet_irreducibles: you write len(self.upper_covers()) which calls upper_covers_iterator which iterates over all out-neighbors of a vertex of the HasseDiagram before relabelling them as !Poset elements. Why ? All you want to do is check that the outdegree of those points if 1. You could work on the diagram directly.

  2. Please add the new functions to the index.

  3. You cannot use $ for latex code there. Use ` instead. In order to check that the doc compiles correctly run sage -b && sage -docbuild reference/combinat html and look at the html files produced.

  4. Use () instead of \ to wrap code on multiple lines.

Nathann

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 10, 2014

comment:4

1: But posets.py says "This internal data structure is subject to change at any point. Do not break encapsulation!" But I can do that of course; then I'll also change join_irreducibles.

2: Uh, of course.

3: OK. But then http://www.sagemath.org/doc/developer/coding_basics.html#latex-typesetting needs correcting.

4: OK.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2014

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

b3e1bcaMinor corrections.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2014

Changed commit from 0ee134c to b3e1bca

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Oct 11, 2014

comment:6

3: It seems that $ works in me. Could you check it? 2: There is no index where to add... So, more work to poset documentation project.

As of 1, I did not do that for now; this is direct copy from join_-counterparts. Point 4 is done, but long line of == is still quite hard to read.

Also this same time I removed totally unused argument from cover_relations. Is this OK to do in ticket title saying nothing about it?

@jm58660 jm58660 mannequin added s: needs review and removed s: needs work labels Oct 11, 2014
@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 12, 2014

comment:7

Yo !

1: But posets.py says "This internal data structure is subject to change at any point. Do not break encapsulation!" But I can do that of course; then I'll also change join_irreducibles.

Lost computations for political reasons. This code never ceases to amaze me :-P

3: OK. But then http://www.sagemath.org/doc/developer/coding_basics.html#latex-typesetting needs correcting.

Oh. If it is written in the developper's manual then the nazis can do nothing against you. Good ! :-D

Nathann

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 12, 2014

comment:8

2: There is no index where to add... So, more work to poset documentation project.

Oh, no index for lattices. Right, sorry for my remark !

As of 1, I did not do that for now; this is direct copy from join_-counterparts. Point 4 is done, but long line of == is still quite hard to read.

As you wish. Remember that there is some time there to save if you have time problems later on.

Also this same time I removed totally unused argument from cover_relations. Is this OK to do in ticket title saying nothing about it?

I don't mind. It will depend on the reviewers, though.

I will run all tests on a machine to check that removing this keyword does not break anything somewhere.

Nathann

@tscrim
Copy link
Collaborator

tscrim commented Oct 12, 2014

comment:9

The dollar-sign $ does work, but it is (strongly?) recommended that you use the backticks ` instead. I think there are still issues with the notebook's documentation when using dollar-signs.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2014

Changed commit from b3e1bca to 8b68cdb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2014

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

8b68cdbChanged "$" to "`" as LaTeX marker on documentation.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 13, 2014

comment:11

All tests passed !

Thanks,

Nathann

@vbraun
Copy link
Member

vbraun commented Oct 14, 2014

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