Repository navigation
Add DigraphMaximalQuotients - #959
boriforizs wants to merge 2 commits into
Conversation
|
A bit of a failure in the tests here, as you can see, e.g. at this link. You'll notice in the tst files that there's a list of local variables at the top, and it looks like you need to add (You can add more commits to this PR by simply adding them to the branch and pushing.) |
|
Other things to work on:
Hopefully this gives you some stuff to work on till next meeting. Thanks for emailing me! |
james-d-mitchell
left a comment
james-d-mitchell
left a comment
There was a problem hiding this comment.
A few changes required, then I'll be happy to merge this, thank you @boriforizs !
|
|
||
| # This is to ensure compatibility with AutomorphismGroup and QuotientDigraph. | ||
| if IsMultiDigraph(D) then | ||
| aut := AutomorphismGroup(DigraphRemoveAllMultipleEdges(D)); |
There was a problem hiding this comment.
If D is a mutable digraph, then I think this line will modify it in-place to remove all the multiple edges, probably this can be fixed by just adding DigraphCopyIfMutable(D) here.
| <Ref Oper="QuotientDigraph"/>) obtained by amalgamating precisely two | ||
| vertices of <A>digraph</A>. In particular, each maximal quotient of | ||
| <A>digraph</A> has <C>n - 1</C> vertices. If <A>digraph</A> has fewer | ||
| than two vertices, an empty list is returned. |
There was a problem hiding this comment.
The doc should say something about the complexity of the method, it looks like at least
There was a problem hiding this comment.
Also should mention that these are quotients in the sense of QuotientDigraph so in particular, they can contain loops and multiple edges even if the original digraph does not. Be good to add some tests for this too, i.e. if we have a single node null digraph, and a single node digraph with a loop, then these will both show up in the returned list if I'm not mistaken.
There was a problem hiding this comment.
Also the doc should mention that the returned list is the list of maximal proper quotients up to isomorphism, and not up to equality (also please add test for this too).
| fi; | ||
|
|
||
| # Creating orbit representatives. | ||
| orbs := Orbits(aut, Combinations(DigraphVertices(D), 2), OnSets); |
There was a problem hiding this comment.
This doesn't yet exist in a release version of GAP but we could perhaps use EnumeratorOfCombinations from gap-system/gap#6756 in the future, might be worthwhile mentioning this here in a TODO
| <Returns>A list of digraphs.</Returns> | ||
| <Description> | ||
| If <A>digraph</A> is a digraph with <C>n</C> vertices, then this | ||
| attribute returns a list containing all maximal quotients of |
There was a problem hiding this comment.
| attribute returns a list containing all maximal quotients of | |
| attribute returns a list containing all maximal proper quotients of |
No description provided.