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
Kautz, Imase and Itoh, and Generalized de Bruijn digraph generators #12626
Comments
Author: David Coudert |
comment:1
The implementation of the Kautz generator can certainly be done in a more elegant way, but I don't know how. |
This comment has been minimized.
This comment has been minimized.
comment:2
Update of the patch removing the documentation warnings caused by duplicated references and an inline literal that was starting at a line and ending at the next one. It should now pass all tests without warnings... |
comment:3
Helloooooo David !!! Looks like there is something wrong with your doctests ! You forgot to add many "::" and so the doctests you add do not appear in the documentation (and probably are not tested when you run "sage -t") Some other things : we try to keep the "first line" of the documentation of each function "short and descriptive". Something like "its meaning in at most one line". Something like what you find in "RandomDirectedGNR" or in "DeBruijn". When it requires some details, they are given immediately after, though. In the documentation you define the "degree d", but "degree" appears in the formulas, like in "v \equiv -u*degree-a-1 \mod{n}" I noticed that you added a link toward a Wikipedia page... Did you see this ":wikipedia:" somewhere already ? During the last Sage Combinat days Florent Hivert actually added such a thing in Sage ! It is patch #12490. As written, this link does not display correctly in the doc, but since #12490 has been merged you can replace
by
Note that the argument is not a full link, but only the article's name Nathann |
comment:4
I have addressed all points and I can see all doctests in the documentation. Should be better now. Thanks Nathann ! |
comment:5
Helloooo David !!! Well, I only have two unimportant things to bring up, and a question :
I also spent some time playing with the code of the Kautz constructor. I have to admit I am not a big fan of doing all the computations through words, if you end up casting them to strings in the end. I don't think it would be a good idea to keep words in the graph either, as then the vertices would appear as "word: 1" instead of '1', hence it is not a good idea either. If you only need the is_suffix method you can just use string methods :
This would mean that all of your symbols have a 'width' of 1, though. Hence, "Bb" could not be a symbol as in your examples. The other thing is that as such, it is not possible to "decompose" a vertex of your graph. Vertices are strings of symbols split by commas, but you have no idea what the "first symbol of the word" is. The answer to that would be to use tuples ("Bb", '1') to encode the vertices, but of course they would be longer. All this to say that I do not know of any "good solution" to the problem. I'm just bringing this up for you to decide, after all you know better than I do what these graphs can be used for Oh, and.. I would have changed
by
to make clearer that it is an equivalent definition. That again is up to you ! Nathann |
Attachment: trac_12626_kautz_digraph_generator.patch.gz |
comment:6
Nathann, I have done all corrections. Concerning implementation choices for the Kautz digraph generator. I tried to be consistent with existing implementation of the DeBruijn digraph generator. I also used ideas from the ButterflyGraph generator. So we have the same behavior:
To be even more consistent, I have now modified the DeBruijn Generator to add the option of using integer vertex labels. I have also corrected the wikipedia link of the documentation of the DeBruijn generator.
I agree that this implementation is not perfect, but it has the merit of being consistent with previous implementation choices for the de Bruijn generator. Since I don't how this generator is used, I prefer to let it as it is. Of course, most of the users will use either a normal alphabet with simple symbols, or the integer version. In fact, to be more general, we should be able to provide permutation functions (on the alphabet, and on the shifting) to the deBruijn generator, but this is another story (see http://dx.doi.org/10.1002/net.10043 or my PhD thesis for more details) and it is not working for the Kautz graph... Thank you again, David. |
comment:7
Got it ! Anyway one can still use "split" to find the decomposition back, or remove all commas to deal with words... Nathann |
Reviewer: Nathann Cohen |
comment:8
And a good news : patch #12224 should be available at some point, which according to Vincent will change the default display of Words from "Word: aaabbb" to "aaabbb". So from this point we will be able to use graphs defined on Words, and so to update those methods and the deBuijn graph too Nathann |
comment:9
That would be nice and ease the implementation of various algorithms on the deBruijn/Kautz digraphs: paths computation, ... D. |
Merged: sage-5.0.beta8 |
This patch implements generators for Kautz digraphs, Imase and Itoh digraphs, and Generalized de Bruijn digraphs.
CC: @nathanncohen
Component: graph theory
Author: David Coudert
Reviewer: Nathann Cohen
Merged: sage-5.0.beta8
Issue created by migration from https://trac.sagemath.org/ticket/12626
The text was updated successfully, but these errors were encountered: