-
Notifications
You must be signed in to change notification settings - Fork 66
Implement IsBetweenCoverAndLattice #920
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
382d52d
cd90e4d
ca5bc24
e7db279
f1af22b
662290c
d2c54bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -91,6 +91,60 @@ function(D, P, U, join) | |||||||||
| return tab; | ||||||||||
| end); | ||||||||||
|
|
||||||||||
| # Variant of DIGRAPHS_MeetJoinTable that does not require the input digraph | ||||||||||
| # to have all transitive edges: reachability between previously processed | ||||||||||
| # vertices is checked via the table being constructed, rather than via | ||||||||||
| # direct edge queries on the input digraph. | ||||||||||
| # Note: above descriptive comment is LLM-generated. | ||||||||||
| BindGlobal("DIGRAPHS_MeetJoinTableBetweenCover", | ||||||||||
| function(N, P, U, join) | ||||||||||
| local ord, tab, S, i, x, T, l, q, z, y; | ||||||||||
|
|
||||||||||
| tab := List([1 .. N], x -> []); | ||||||||||
|
|
||||||||||
| ord := []; | ||||||||||
| for i in [1 .. N] do | ||||||||||
| ord[P[i]] := i; | ||||||||||
| od; | ||||||||||
|
|
||||||||||
| S := []; | ||||||||||
|
|
||||||||||
| for x in P do | ||||||||||
| tab[x, x] := x; | ||||||||||
| for y in S do | ||||||||||
| T := []; | ||||||||||
| for z in U[x] do | ||||||||||
| Add(T, tab[y, z]); | ||||||||||
| od; | ||||||||||
| T := Set(T); | ||||||||||
| l := Length(T); | ||||||||||
| if l = 0 then | ||||||||||
| return fail; | ||||||||||
| fi; | ||||||||||
| q := T[l]; | ||||||||||
| for i in [1 .. l - 1] do | ||||||||||
| z := T[i]; | ||||||||||
| if ord[z] > ord[q] then | ||||||||||
| q := z; | ||||||||||
| fi; | ||||||||||
| od; | ||||||||||
| for z in T do | ||||||||||
| # the below conditions are the only part that | ||||||||||
| # differs from MeetJoinTable above | ||||||||||
| if join and tab[q, z] <> z then | ||||||||||
| return fail; | ||||||||||
| elif not join and tab[z, q] <> z then | ||||||||||
| return fail; | ||||||||||
|
Comment on lines
+132
to
+137
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that this is the only difference, would it be feasible to modify
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On it! Would doing a text search for "DIGRAPHS_MeetJoinTable" be sufficient to ensure that all places affected by the function signature breaking change are covered?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I think so! It would also be nice if you could add a comment describing what each of the input parameters |
||||||||||
| fi; | ||||||||||
| od; | ||||||||||
| tab[x, y] := q; | ||||||||||
| tab[y, x] := q; | ||||||||||
| od; | ||||||||||
| Add(S, x); | ||||||||||
| od; | ||||||||||
| return tab; | ||||||||||
| end); | ||||||||||
|
|
||||||||||
| InstallMethod(DIGRAPHS_IsJoinSemilatticeAndJoinTable, "for a digraph", | ||||||||||
| [IsDigraph], | ||||||||||
| function(D) | ||||||||||
|
|
@@ -137,6 +191,41 @@ InstallMethod(IsMeetSemilatticeDigraph, "for a digraph", | |||||||||
| [IsDigraph], | ||||||||||
| D -> DIGRAPHS_IsMeetSemilatticeAndMeetTable(D)[1]); | ||||||||||
|
|
||||||||||
| InstallMethod(IsBetweenCoverAndLattice, "for a digraph", | ||||||||||
| [IsDigraph], | ||||||||||
| function(D) | ||||||||||
| local copy, order, hasse, neighbours, table; | ||||||||||
|
|
||||||||||
|
ThatOtherAndrew marked this conversation as resolved.
Outdated
|
||||||||||
| # 1. Topologically sort the nodes in D. | ||||||||||
| copy := DigraphRemoveLoops(DigraphMutableCopyIfMutable(D)); | ||||||||||
| # ^ protect from nasty mutable side effects i think | ||||||||||
|
ThatOtherAndrew marked this conversation as resolved.
Outdated
ThatOtherAndrew marked this conversation as resolved.
Outdated
|
||||||||||
| order := DigraphTopologicalSort(copy); | ||||||||||
| if order = fail then | ||||||||||
| return false; | ||||||||||
|
ThatOtherAndrew marked this conversation as resolved.
|
||||||||||
| fi; | ||||||||||
|
|
||||||||||
| # 2. Iterate through pairs of nodes of D in topological order | ||||||||||
| # and construct a table of their meets. | ||||||||||
| hasse := DigraphTransitiveReduction(copy); | ||||||||||
| neighbours := InNeighbours(hasse); | ||||||||||
| table := DIGRAPHS_MeetJoinTableBetweenCover( | ||||||||||
| DigraphNrVertices(copy), | ||||||||||
| Reversed(order), | ||||||||||
| neighbours, | ||||||||||
| false); | ||||||||||
| if table = fail then | ||||||||||
| return false; | ||||||||||
| fi; | ||||||||||
|
|
||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
We can save the result as the meet table here, since we already did all that work to compute it. The same should be done for the join table below. |
||||||||||
| neighbours := OutNeighbours(hasse); | ||||||||||
| table := DIGRAPHS_MeetJoinTableBetweenCover( | ||||||||||
| DigraphNrVertices(copy), | ||||||||||
| order, | ||||||||||
| neighbours, | ||||||||||
| true); | ||||||||||
| return table <> fail; | ||||||||||
|
Comment on lines
+221
to
+227
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the similarity of this line to the previous one, would it might make more sense to have two functions |
||||||||||
| end); | ||||||||||
|
|
||||||||||
| InstallImmediateMethod(IsStronglyConnectedDigraph, | ||||||||||
| IsDigraph and HasDigraphStronglyConnectedComponents, 0, | ||||||||||
| D -> Length(DigraphStronglyConnectedComponents(D).comps) = 1); | ||||||||||
|
|
||||||||||
Uh oh!
There was an error while loading. Please reload this page.