Opened 8 years ago
Closed 8 years ago
#18515 closed enhancement (fixed)
LatticePoset: add sublattice()
Reported by:  jmantysalo  Owned by:  

Priority:  major  Milestone:  sage6.8 
Component:  combinatorics  Keywords:  
Cc:  chapoton, ncohen  Merged in:  
Authors:  Jori Mäntysalo, Nathann Cohen  Reviewers:  Nathann Cohen, Jori Mäntysalo 
Report Upstream:  N/A  Work issues:  
Branch:  a86919a (Commits, GitHub, GitLab)  Commit:  a86919aedb982998f23bd2c6eaf523caf1562c27 
Dependencies:  Stopgaps: 
Description
Add a function to get smallest sublattice of the lattice containing given elements.
Change History (12)
comment:1 Changed 8 years ago by
Branch:  → u/jmantysalo/latticeposet__add_sublattice__ 

comment:2 Changed 8 years ago by
Authors:  → Jori Mäntysalo 

Cc:  chapoton ncohen added 
Commit:  → 54d912c9e08792b278a6e86b37a572f87f27746f 
Status:  new → needs_review 
comment:3 Changed 8 years ago by
Commit:  54d912c9e08792b278a6e86b37a572f87f27746f → 34a588d1878d613b8e36736a625be1e49d67be77 

Branch pushed to git repo; I updated commit sha1. New commits:
34a588d  Arghs, should return a lattice, not a poset.

comment:4 Changed 8 years ago by
Branch:  u/jmantysalo/latticeposet__add_sublattice__ → u/deinst/latticeposet__add_sublattice__ 

comment:5 Changed 8 years ago by
Commit:  34a588d1878d613b8e36736a625be1e49d67be77 → 47f2ce7a7799982ea29157c932bf91b31b78a3cc 

I think there is a typo. I've changed the code to what I think is correct. The previous code did the following:
sage: L = Posets.BooleanLattice(3) sage: L.sublattice([3,5,6,7])  ValueError Traceback (most recent call last) <ipythoninput14837853635c8a> in <module>() > 1 L.sublattice([Integer(3),Integer(5),Integer(6),Integer(7)]) /Users/davideinstein/projects/sage/local/lib/python2.7/sitepackages/sage/combinat/posets/lattices.pyc in sublattice(self, elms) 881 elms.append(m) 882 n=True > 883 return LatticePoset(self.subposet(elms)) 884 885 ############################################################################ /Users/davideinstein/projects/sage/local/lib/python2.7/sitepackages/sage/combinat/posets/lattices.pyc in LatticePoset(data, *args, **options) 406 P = Poset(data, *args, **options) 407 if not P.is_lattice(): > 408 raise ValueError("Not a lattice.") 409 410 return FiniteLatticePoset(P, category = FiniteLatticePosets(), facade = P._is_facade) ValueError: Not a lattice.
Also, could you put whitespace around =
as suggested by http://doc.sagemath.org/html/en/developer/coding_basics.html#pythoncodestyle
New commits:
47f2ce7  Fixed probable typo

comment:7 followup: 8 Changed 8 years ago by
Thanks! And yes, there was a stupid typo. I'll try to remember spaces from now on. (A promise I have made before...)
So, who's going to review this?
comment:8 Changed 8 years ago by
Yo !
I added a commit at public/18515
which improves the algorithm a bit. In general, try to avoid using 'Subsets' (or any combinat stuff) if you can avoid it: they implement their code to deal with infinitely more abstraction than you usually need (categories, parents, elements, ...).
sage: type(Subsets([1,2,3],2).an_element()) <class 'sage.sets.set.Set_object_enumerated_with_category'>
When you only need to enumerate pairs of points, the better is to use itertools:
sage: from itertools import combinations sage: list(combinations([1,2,3],2)) [(1, 2), (1, 3), (2, 3)]
Simple, and more efficient.
So, who's going to review this?
We can review each other's commits, it is not forbidden. For as long as everything is doubled checked, no problem.
Nathann
comment:9 Changed 8 years ago by
Seems good, but of course doesn't make that much difference than totally other algorithm mentioned in the note. I'll check the code of Nathann and then put this to positive review. (Assuming that deinst doesn't do it before that.)
comment:10 Changed 8 years ago by
Authors:  Jori Mäntysalo → Nathann Cohen 

Branch:  u/deinst/latticeposet__add_sublattice__ → public/18515 
Commit:  47f2ce7a7799982ea29157c932bf91b31b78a3cc → a86919aedb982998f23bd2c6eaf523caf1562c27 
Reviewers:  → Jori Mäntysalo 
Status:  needs_review → positive_review 
comment:11 Changed 8 years ago by
Authors:  Nathann Cohen → Jori Mäntysalo, Nathann Cohen 

Reviewers:  Jori Mäntysalo → Nathann Cohen, Jori Mäntysalo 
;)
comment:12 Changed 8 years ago by
Branch:  public/18515 → a86919aedb982998f23bd2c6eaf523caf1562c27 

Resolution:  → fixed 
Status:  positive_review → closed 
Slooow. Better than nothing. Needs review.
New commits:
Added a function.
Removed spaces.