Opened 5 years ago
Closed 5 years ago
#20921 closed enhancement (fixed)
LatticePoset: add function to get lattice of sublattices
Reported by:  jmantysalo  Owned by:  

Priority:  major  Milestone:  sage7.3 
Component:  combinatorics  Keywords:  lattice poset 
Cc:  tscrim  Merged in:  
Authors:  Jori Mäntysalo  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  fe8b4f6 (Commits, GitHub, GitLab)  Commit:  fe8b4f61aff1d5f43753c43dfde493c2d2e3eac2 
Dependencies:  Stopgaps: 
Description (last modified by )
Add a function to get the lattice of sublattices.
(First wait #20669 to get integrated at 7.3beta7.)
Change History (22)
comment:1 Changed 5 years ago by
 Description modified (diff)
comment:2 Changed 5 years ago by
 Cc tscrim added
 Keywords lattice poset added; latticeposet removed
comment:3 Changed 5 years ago by
comment:4 followup: ↓ 5 Changed 5 years ago by
In and of itself, yes, "lattice" is ambiguous. However, "latticeposet" is not a word (in English), but having the "lattice poset" together makes it clear. (More of a random note, there are also 2 other distinct nonmathematical uses of the word lattice that comes to mind as well.)
However, you're free to build here upon #20669 before it gets released in a beta.
comment:5 in reply to: ↑ 4 Changed 5 years ago by
Replying to tscrim:
In and of itself, yes, "lattice" is ambiguous. However, "latticeposet" is not a word (in English), but having the "lattice poset" together makes it clear.
But trac does not seem to accept two words keyphrase. Trying to put "foo bar, xyzzy" as keywords does just what "foo, bar, xyzzy" would do. lattice_poset maybe?
However, you're free to build here upon #20669 before it gets released in a beta.
True, but Volker releases betas so often that it makes no big delay.
comment:6 Changed 5 years ago by
 Branch set to u/jmantysalo/latticeposet__add_function_to_get_lattice_of_sublattices
comment:7 Changed 5 years ago by
 Commit set to 85ccdc2ac2b6550d86c5f5aa9db6c59d8bfe6e99
Not ready, but I save it here and will continue on another computer after next beta.
New commits:
85ccdc2  Added function sublattices_lattice.

comment:8 Changed 5 years ago by
 Commit changed from 85ccdc2ac2b6550d86c5f5aa9db6c59d8bfe6e99 to 29f13a12ea8a1429196951835716a4d9181785ca
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
29f13a1  Add sublattices_lattice().

comment:9 Changed 5 years ago by
 Status changed from new to needs_review
comment:10 Changed 5 years ago by
Just pinging... Examples are hard, as even very small lattice can generate a big lattice of sublattices.
comment:11 Changed 5 years ago by
 Reviewers set to Travis Scrimshaw
I would write the input section as:
 ``element_constructor``  string; can be one of the following: * ``'lattice'``  (the default) elements of the lattice will be lattices that correspond to sublattices of ``self`` * ``'tuple'``  elements are tuples of elements of the sublattices of ``self`` * ``'integer'``  the elements are plain integers
Also, it looks like you have an unneeded import of DiGraph
. Otherwise LGTM.
comment:12 Changed 5 years ago by
 Commit changed from 29f13a12ea8a1429196951835716a4d9181785ca to 2863437d07cfc99a28c195826d20515961fb3a49
Branch pushed to git repo; I updated commit sha1. New commits:
2863437  Reviewer comments.

comment:13 Changed 5 years ago by
Corrected those. I used format "original lattice", as self
is not very much used in docstring in this file.
Should it be "elements" or "the elements"?
comment:14 followup: ↓ 16 Changed 5 years ago by
Formally (i.e., if a English professor looked at it), I'm not sure if the "the" is necessary here because it is plural and the structure of the sentence is declarative. Feel free to add it as it is not wrong. Also, I missed this above, it should just be (default)
, i.e., no "the" there. Thanks.
comment:15 Changed 5 years ago by
 Commit changed from 2863437d07cfc99a28c195826d20515961fb3a49 to cf557bbcdbe1e6d8a306d586a6cd3820c145344c
Branch pushed to git repo; I updated commit sha1. New commits:
85ccdc2  Added function sublattices_lattice.

cf7f1e5  Merge branch 'u/jmantysalo/latticeposet__add_function_to_get_lattice_of_sublattices' of git://trac.sagemath.org/sage into t/20921/latticeposet__add_function_to_get_lattice_of_sublattices

cf557bb  Removed a 'the'.

comment:16 in reply to: ↑ 14 ; followup: ↓ 17 Changed 5 years ago by
Replying to tscrim:
Formally (i.e., if a English professor looked at it), I'm not sure if the "the" is necessary here because it is plural and the structure of the sentence is declarative. Feel free to add it as it is not wrong.
If it is not needed, I can left it off. I was just wondering when you wrote "tuple  elements are" vs. "integer  the elements".
Ready now?
comment:17 in reply to: ↑ 16 Changed 5 years ago by
 Status changed from needs_review to positive_review
Replying to jmantysalo:
Replying to tscrim:
Formally (i.e., if a English professor looked at it), I'm not sure if the "the" is necessary here because it is plural and the structure of the sentence is declarative. Feel free to add it as it is not wrong.
If it is not needed, I can left it off. I was just wondering when you wrote "tuple  elements are" vs. "integer  the elements".
Whoops, my bad.
Ready now?
Yep. Thanks.
comment:18 Changed 5 years ago by
 Status changed from positive_review to needs_work
sage t long src/sage/combinat/posets/lattices.py ********************************************************************** File "src/sage/combinat/posets/lattices.py", line 1743, in sage.combinat.posets.lattices.FiniteLatticePoset.sublattices_lattice Failed example: sll = L.sublattices_lattice() Exception raised: Traceback (most recent call last): File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 499, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 862, in compile_and_execute exec(compiled, globs) File "<doctest sage.combinat.posets.lattices.FiniteLatticePoset.sublattices_lattice[4]>", line 1, in <module> sll = L.sublattices_lattice() File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/lattices.py", line 1868, in sublattices_lattice G.relabel(lambda x: self.sublattice(x)) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/graphs/generic_graph.py", line 19817, in relabel perm = dict( [ i, perm(i) ] for i in self.vertices() ) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/graphs/generic_graph.py", line 19817, in <genexpr> perm = dict( [ i, perm(i) ] for i in self.vertices() ) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/lattices.py", line 1868, in <lambda> G.relabel(lambda x: self.sublattice(x)) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/lattices.py", line 1680, in sublattice return LatticePoset(self.subposet(current_set)) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/posets.py", line 4513, in subposet elements = [self(e) for e in elements] File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/posets.py", line 1232, in __call__ return super(FinitePoset, self).__call__(element) File "sage/structure/parent.pyx", line 1107, in sage.structure.parent.Parent.__call__ (/home/buildbot/slave/sage_git/build/src/build/cythonized/sage/structure/parent.c:9904) return mor._call_(x) File "sage/structure/coerce_maps.pyx", line 110, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/home/buildbot/slave/sage_git/build/src/build/cythonized/sage/structure/coerce_maps.c:4651) raise File "sage/structure/coerce_maps.pyx", line 105, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/home/buildbot/slave/sage_git/build/src/build/cythonized/sage/structure/coerce_maps.c:4539) return C._element_constructor(x) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/posets.py", line 1208, in _element_constructor_ % type(element)) ValueError: <type 'int'> is not an element of this poset ********************************************************************** File "src/sage/combinat/posets/lattices.py", line 1844, in sage.combinat.posets.lattices.FiniteLatticePoset.sublattices_lattice Failed example: sll = L.sublattices_lattice() Exception raised: Traceback (most recent call last): File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 499, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 862, in compile_and_execute exec(compiled, globs) File "<doctest sage.combinat.posets.lattices.FiniteLatticePoset.sublattices_lattice[4]>", line 1, in <module> sll = L.sublattices_lattice() File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/lattices.py", line 1868, in sublattices_lattice G.relabel(lambda x: self.sublattice(x)) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/graphs/generic_graph.py", line 19817, in relabel perm = dict( [ i, perm(i) ] for i in self.vertices() ) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/graphs/generic_graph.py", line 19817, in <genexpr> perm = dict( [ i, perm(i) ] for i in self.vertices() ) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/lattices.py", line 1868, in <lambda> G.relabel(lambda x: self.sublattice(x)) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/lattices.py", line 1680, in sublattice return LatticePoset(self.subposet(current_set)) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/posets.py", line 4513, in subposet elements = [self(e) for e in elements] File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/posets.py", line 1232, in __call__ return super(FinitePoset, self).__call__(element) File "sage/structure/parent.pyx", line 1107, in sage.structure.parent.Parent.__call__ (/home/buildbot/slave/sage_git/build/src/build/cythonized/sage/structure/parent.c:9904) return mor._call_(x) File "sage/structure/coerce_maps.pyx", line 110, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/home/buildbot/slave/sage_git/build/src/build/cythonized/sage/structure/coerce_maps.c:4651) raise File "sage/structure/coerce_maps.pyx", line 105, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/home/buildbot/slave/sage_git/build/src/build/cythonized/sage/structure/coerce_maps.c:4539) return C._element_constructor(x) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/combinat/posets/posets.py", line 1208, in _element_constructor_ % type(element)) ValueError: <type 'int'> is not an element of this poset ********************************************************************** File "src/sage/combinat/posets/lattices.py", line 1856, in sage.combinat.posets.lattices.FiniteLatticePoset.sublattices_lattice Failed example: sll.is_isomorphic(Posets.BoolenLattice(3)) Exception raised: Traceback (most recent call last): File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 499, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 862, in compile_and_execute exec(compiled, globs) File "<doctest sage.combinat.posets.lattices.FiniteLatticePoset.sublattices_lattice[10]>", line 1, in <module> sll.is_isomorphic(Posets.BoolenLattice(Integer(3))) AttributeError: type object 'Posets' has no attribute 'BoolenLattice' ********************************************************************** 1 item had failures: 3 of 24 in sage.combinat.posets.lattices.FiniteLatticePoset.sublattices_lattice [310 tests, 3 failures, 0.75 s]
comment:19 Changed 5 years ago by
 Commit changed from cf557bbcdbe1e6d8a306d586a6cd3820c145344c to fe8b4f61aff1d5f43753c43dfde493c2d2e3eac2
Branch pushed to git repo; I updated commit sha1. New commits:
fe8b4f6  Removed duplicate function.

comment:20 Changed 5 years ago by
 Status changed from needs_work to needs_review
AARGHS! There was my temporary copy still in the branch. Now it is removed and tests passed.
comment:21 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:22 Changed 5 years ago by
 Branch changed from u/jmantysalo/latticeposet__add_function_to_get_lattice_of_sublattices to fe8b4f61aff1d5f43753c43dfde493c2d2e3eac2
 Resolution set to fixed
 Status changed from positive_review to closed
Isn't "lattice" an ambiguous keyword? It refers to two different things, try to click it and see.