Implementation of concepts related to fundamental theorem of semidistributive lattices - #42593
Implementation of concepts related to fundamental theorem of semidistributive lattices#42593Hey-Ya7 wants to merge 17 commits into
Conversation
|
Le plugin "Lint" est pas content, merci de corriger tout ca. |
|
Documentation preview for this PR (built with commit 76a6204; changes) is ready! 🎉 |
dcoudert
left a comment
There was a problem hiding this comment.
Thank you for contributing Sagemath.
However, I don't think these methods should be added to DiGraph. These methods are very specific to lattices and so should be somewhere in src/combinat/posets/....
Below are some commets on your code. Similar comments apply to method is_two_acyclic_factorization_system.
|
|
||
| - ``X`` -- Set; a subset of vertices | ||
|
|
||
| OUTPUT: The right orthogonal of X as a set of vertices. |
| ....: G = digraphs.RandomDirectedGNP(10, .3) | ||
| ....: assert G.right_orthogonal(set(G)) == set() | ||
| """ | ||
| l = set(self.vertices()) |
There was a problem hiding this comment.
you can use l = set(self) (several times in your code)
|
|
||
| - ``X`` -- Set; a subset of vertices | ||
|
|
||
| OUTPUT: The left orthogonal of X as a set of vertices. |
| G = DiGraph() | ||
| from sage.sets.set import Set | ||
| G.add_vertex(Set(self.vertices())) | ||
| pairs = {Set(self.vertices()): Set()} # dictionary of pairs, where |
There was a problem hiding this comment.
place the comments above the declaration of pairs. It makes it easier to respect the 80 columns mode.
There was a problem hiding this comment.
also, please avoid Set in library code.
There was a problem hiding this comment.
I will need to use some kind of set-like object that is immutable as the algorithm involves indexing a dictionary by some right orthogonal set, would frozenset() be preferred in this case, or is that also depreciated?
| pairs = {Set(self.vertices()): Set()} # dictionary of pairs, where | ||
| # the first component is indexed by the second | ||
| # for example, pairs[second_term] should give first_term | ||
| next_pairs = [Set(self.vertices())] |
There was a problem hiding this comment.
- next_pairs = [Set(self.vertices())]
- while next_pairs != []:
+ next_pairs = [Set(self)]
+ while next_pairs:
new_pairs = []
for rt in next_pairs:
covering_pairs = []
- for x in self.vertex_iterator():
+ for x in self:
if x in pairs[rt]:
continue
# calculate the new right orthogonal by removing vertices
new_rt = rt.difference(self.neighbors_out(x))
new_rt = new_rt.difference([x])
covering_pairs.append(new_rt)
if new_rt in pairs:
# merge all left components with the same right orthogonal
pairs[new_rt] = pairs[new_rt].union(pairs[rt])
else:
pairs[new_rt] = pairs[rt]
new_pairs.append(new_rt)
pairs[new_rt] = pairs[new_rt].union(Set({x}))
# generate the upper covers
for new_rt in covering_pairs:
if rt != new_rt:
- G.add_vertex(new_rt)
G.add_edge(rt, new_rt)
next_pairs = new_pairs
if labels == "left":
G.relabel(lambda v: pairs[v])
elif labels != "right":
G.relabel(lambda v: (pairs[v], v))
from sage.combinat.posets.lattices import LatticePoset
return LatticePoset(G)| sage: G.surjective_edges(loops=True) | ||
| [] | ||
| """ | ||
| E = [] |
There was a problem hiding this comment.
E = []
for x, y in self.edge_iterator(labels=False):
if x != y:
if all(self.has_edge(x, z) for z in self.neighbors_out(y)):
E.append((x, y))
elif loops:
E.append((x, y))
return E| sage: G.injective_edges(loops=True) | ||
| [] | ||
| """ | ||
| E = [] |
There was a problem hiding this comment.
E = []
for y, z in self.edge_iterator(labels=False):
if y != z:
if all(self.has_edge(x, z) for x in self.neighbors_in(y)):
E.append((y, z))
elif loops:
E.append((y, z))
return E|
We now have a category of Semidistributive lattices in place, in src/sage/categories/lattice_posets.py. Maybe this would be the place to put some of these as methods of semidistributive lattices ? |
|
Thank you very much for your comments @dcoudert, I have modified my code following your suggestions; however I am unsure where to put these methods. I agree that these methods should probably be somewhere in the combinat/posets folder, but there doesn't really seem to be an appropriate file to add these to - I don't think these would fit well in neither posets.py nor lattices.py as they don't make sense as poset/lattice/semilattice methods. I am currently considering to perhaps create a new file under combinat/posets and transfer these methods there, and changing them from class methods to functions taking a DiGraph object as argument. Would this be an acceptable way to implement these changes? |
|
Oui, an independant file somewhere in combinat/posets seems to be a reasonable idea. It could be named "semidistributivity.py" maybe. |
|
there is a wrong change in the file digraph.py |
|
you can try to sort the doctest to make it reproducible, or just check the length |
|
Sorting the list seems to have fixed it, although I'm not sure what's causing the [g-o] test error. |
|
the failure in libs/ecl is unrelated and happens currently in most pull requests |
|
|
||
| sage: from sage.combinat.posets.semidistributivity import is_two_acyclic_factorization_system | ||
| sage: G = DiGraph([(0, 1), (1, 0), (0, 0), (1, 1)], loops=True) | ||
| sage: is_two_acyclic_factorization_system(G, certificate=True) # (0, 1), (1, 0) are both surjective |
There was a problem hiding this comment.
please do not use comments, but write them in documentation main text
|
J'ai fait quelques suggestions. Il faut les ajouter au "batch" puis "commiter l'ensemble" |
Co-authored-by: Frédéric Chapoton <chapoton@unistra.fr>
This PR implements key concepts from Reading, Speyer, and Thomas' 2024 paper, "The fundamental theorem of finite semidistributive lattices" . Specifically, it adds the following methods to the file src/sage/graphs/digraph.py :
The main motivation behind all this is the Fundamental Theorem of Semidistributive Lattices, which says that any finite semidistributive lattice is isomorphic to some lattice of orthogonal pairs of a two-acyclic factorization system.
📝 Checklist
⌛ Dependencies
I am unaware of any PRs that this implementation depends on.