Opened 8 years ago
Closed 8 years ago
#18515 closed enhancement (fixed)
LatticePoset: add sublattice()
Reported by: | jmantysalo | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.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) <ipython-input-14-837853635c8a> in <module>() ----> 1 L.sublattice([Integer(3),Integer(5),Integer(6),Integer(7)]) /Users/davideinstein/projects/sage/local/lib/python2.7/site-packages/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/site-packages/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#python-code-style
New commits:
47f2ce7 | Fixed probable typo
|
comment:7 follow-up: 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.