train-tracks

[5e1794c3-77c0-453a-bf39-4b14c1cb833c]

We propose to implement in Sage the train-tracks package developed by Thierry Coulbois:

The main feature and the main achievement of the program is to compute train-track representative for (outer) automorphisms of free groups. phi.train track() computes a train-track representative for the (outer) automorphism phi. This train-track can be either an absolute train-track or a relative train-track. The celebrated theorem of Bestvina and Feighn assures that if phi is fully irreducible (iwip), then there exists an absolute train-track representing phi. The train-track(relative=False) method will terminate with either an absolute train-track or with a topological representative with a reduction: an invariant strict subgraph with non-trivial fundamental group. One more feature of train-tracks (absolute or relative) is to lower the number of Nielsen paths. Setting the stable=True option will return a train-track with at most one indivisible Nielsen path (per exponential stratum if it is a relative train-track).

See also:

• https://github.com/coulbois/sage-train-track
• https://www.i2m.univ-amu.fr/perso/thierry.coulbois/train-track/

• 31223: Add sage-train-track as an optional package

CC: @tscrim @sagetrac-tmonteil @videlec

Component: group theory

Keywords: free-group automorphism

Author: Dominique Benielli, Thierry Coulbois

Branch/Commit: public/train-track @ dcc540c

Issue created by migration from https://trac.sagemath.org/ticket/20154

[videlec]

comment:76

Hi Thierry, Hi Dominique,

The behavior is certainly documented in GAP. It might be worth to also have it (and test it) inside Sage. I am looking at the other pieces of your code now.

Vincent

[fchapoton]

comment:77

Do you really need compare_letters ? Recall that cmp(...) is going to disappear in python3.

[d53c3c1f-61cc-4040-be1f-8ec5f87eb3a8]

comment:78

You mean the compare_letters in AlphabetWithInverses ?

Well, we really need to be carefull on the order of the alphabet. And I don't see a way around that. We need to have alphabets where each letter comes with an inverse, we need to be able to add and remove letters.

I agree that this is inspired by old versions of Words and Alphabet which now have disappeared from Sage.

The AlphabetWithInverses is now only used by our InverseGraph (and not by FreeGroup), and we want to strictly comply with Serre's definition (see Arbres, amlagames, SL2, aka Trees).

[fchapoton]

comment:79

there is only one compare_letters in the git branch, and it seems not to be used anywhere (only appears in its own doc)

sorting should now generally be done using the keyword key= instead of cmp=

[videlec]

comment:80

The compare_letters might be indirectly used in some word algorithm (though I doubt it).

[fchapoton]

comment:81

there is no compare_letters anywhere else in sage

(git grep "compare_letters")

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

653a251

compare_letters() removed as it is never used

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from cd64177 to 653a251

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

ae9a96f

Minor correction in the doc of FreeGroupAutomorphism.train_track() (due to my French mother tongue, more than two meant two or more + correction in the doc of TrainTrackMap.stabilize() to be more clear that the output is the folding map (not the train-track map) + one correction for a case where that method returned False instead of the identity map

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 653a251 to ae9a96f

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

d92640a

Merge branch 'public/train-track' in 7.5.b2

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from ae9a96f to d92640a

[fchapoton]

comment:85

does not apply

[5e1794c3-77c0-453a-bf39-4b14c1cb833c]

comment:86

I sent a question to https://ask.sagemath.org/question/35988/patchboot-coverage-failure-and-review/ because I do not know what to do to fix it.

Hi sage team, My ticket #20154: train-tracks in public/train-track, is in need-work status, I guess the issue comes from coverage. But I do not see the fie ext/interpreters/wrapper_cc.pyx in my distribution , and for me this file doesn't belong to the ticket (that was true previously whent the patchboot works). I have absolutly no idea how to fix it, could you please tell me what to do to fix it and succes the review.

Thanks Dominique

========== coverage ========== git checkout patchbot/ticket_merged Already on 'patchbot/ticket_merged' Missing doctests in ext/interpreters/wrapper_cc.pyx: 0 / 2 = 0% Full doctests in groups/free_groups/convex_core.py: 18 / 18 = 100% Full doctests in groups/free_groups/free_group.py: 22 / 22 = 100% Full doctests in groups/free_groups/free_group_automorphism.py: 61 / 61 = 100% Full doctests in groups/free_groups/graph_map.py: 18 / 18 = 100% Full doctests in groups/free_groups/graph_self_map.py: 41 / 41 = 100% Full doctests in groups/free_groups/inverse_alphabet.py: 24 / 24 = 100% Full doctests in groups/free_groups/inverse_graph.py: 40 / 40 = 100% Full doctests in groups/free_groups/marked_graph.py: 16 / 16 = 100% Full doctests in groups/free_groups/test_train_track.py: 4 / 4 = 100% Full doctests in groups/free_groups/train_track_map.py: 23 / 23 = 100% Coverage went from 41463 / 43258 = 95.850% to 41711 / 43508 = 95.870%

[fchapoton]

comment:87

This kind of patchbot report you can safely ignore. The patchbot is not perfect, and sometimes gives wrong failures. Patchbot is only here to help, not to be taken as an absolute requirement.

The problem now is that you need to rebase your branch on the latest beta, because there is a conflict (this make the branch red at top of the present page)

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

3bd8e1c	tests failuures
143eb01	test coverage
d53d06a	try foreign latex failure
8551333	remove _cmp
6774b7f	remove __cmp and tests induced
283c702	compare_letters() removed as it is never used
fd8ddec	Minor correction in the doc of FreeGroupAutomorphism.train_track() (due to my French mother tongue, more than two meant two or more + correction in the doc of TrainTrackMap.stabilize() to be more clear that the output is the folding map (not the train-track map) + one correction for a case where that method returned False instead of the identity map
c060ab1	small typo
c4a2309	fusion
48df86f	merge bug

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from d92640a to 48df86f

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 48df86f to 514cff1

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

514cff1

correction for python3

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 514cff1 to d9256f7

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

d9256f7

nothing

[5e1794c3-77c0-453a-bf39-4b14c1cb833c]

comment:91

I think I have corrected all python3 troubles, even though the patchboot did not give new result since 2016-12-14, I think it ok for review

[fchapoton]

comment:92

Because you are not yet trusted, I have to launch my personal bot manually.

Result:

• some failing doctests
• some bad unicode character (maybe add uft8 declaration at top of file ?)
• need to replace <type 'str'> by <... 'str'> for python3 compatibility

see patchbot report for more

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from d9256f7 to 9f3c92f

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

9f3c92f

pthshoot coorected

[jhpalmieri]

comment:94

You should rebase this on the most recent version of Sage: I think the doctest failures are because of new code that uses free groups and imports it from its old location, leading to deprecation warnings.

[5e1794c3-77c0-453a-bf39-4b14c1cb833c]

comment:95

Many thanks for your help, I corrected the bad UTF8 unicode from python 2. And its seems to work with the new rebase version too. I cross fingers my last commit pass the pathboot, and the ticket can pass.

[fchapoton]

comment:96

does not apply

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

0bd5e41	Merge branch 'public/train-track' of git://trac.sagemath.org/sage into public/train-track
1252591	Doing some doc improvements.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 9f3c92f to 1252591

[tscrim]

comment:98

I've done the rebase to the current Sage version and did a little bit of doc cleanup and other small tweaks. It still needs a complete doc readthrough (I maybe got through 30%). There are some references that seem like they needed to be added.

[5e1794c3-77c0-453a-bf39-4b14c1cb833c]

comment:99

Hi,

Thanks for your update for the new version 8, Unfortunately my version obtained by pull is not working, I do not understand what happens.

Otherwise I would like to succes the review, and I would know what is missing, and I can do.

Thanks for help

Dominique

PS error message of doc build

Error building the documentation. Traceback (most recent call last): File "/home/dominique/projets/sage/local/lib/python2.7/runpy.py", line 174, in _run_module_as_main "main", fname, loader, pkg_name) File "/home/dominique/projets/sage/local/lib/python2.7/runpy.py", line 72, in _run_code exec code in run_globals File "/home/dominique/projets/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/main.py", line 2, in main() File "/home/dominique/projets/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/init.py", line 1640, in main builder() File "/home/dominique/projets/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/init.py", line 510, in _wrapper build_many(build_ref_doc, L) File "/home/dominique/projets/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/init.py", line 266, in build_many results.append(target(arg)) File "/home/dominique/projets/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/init.py", line 70, in build_ref_doc getattr(ReferenceSubBuilder(doc, lang), format)(*args, *kwds) File "/home/dominique/projets/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/init.py", line 720, in _wrapper getattr(DocBuilder, build_type)(self, args, **kwds) File "/home/dominique/projets/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/init.py", line 104, in f runsphinx() File "/home/dominique/projets/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/sphinxbuild.py", line 215, in runsphinx raise exception OSError: [manifolds] /home/dominique/projets/sage/src/doc/en/reference/manifolds/index.rst:4: WARNING: undefined label: tensors-on-free-modules (if the link has no caption the label must precede a section header)

[tscrim]

comment:100

Try running make doc-clean && make.

One of the most tedious things is to bring the doc formating similar to the changes I made (see also the sage dev guide). I will try to go through the code itself soon too.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

747f504

minor typos in docstring corrected

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 1252591 to 747f504

[5e1794c3-77c0-453a-bf39-4b14c1cb833c]

comment:102

Many thanks you very munch for your upgrade,

the make doc-clean && make is now ok

Do you think we can go back on need review status?

[tscrim]

comment:103

Someone still needs to go through and standardize the docstrings to use Sage's conventions (comment:100). In addition to the changes I made, you can also use the Sage conventions page, although the INPUT: items should not end in periods/full stops/. (unless they are really long) and you do not need an INPUT: block if there is no input (self does not count) or an OUTPUT: if the output is clear from the 1-line doc. I will go back to looking through the code once that is done.

This also needs a (likely trivial) rebase on the latest develop version.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 747f504 to 3a4bf05

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

3a4bf05

minor typo

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 3a4bf05 to c1805db

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

c1805db

merge with develop branch

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from c1805db to b81e62d

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

b81e62d

INPUT end . correction

[5e1794c3-77c0-453a-bf39-4b14c1cb833c]

comment:107

Hi,

I done the rebase and the correction for INPUT intems end with . only where there are long. Tell me if it is ok for a need review now

Thanks Dominique

[tscrim]

comment:108

Still a lot of doc issues (see also comment:19):

• You need to replace $ with `.
• What does \index{Nielsen path} do? I think it will not do what you want with the build documentation.
• Are the macros \CVN and \FN defined? If not, you probably want to replace them with the corresponding true Latex code (rather than add to the macro list).
• Use \ZZ instead of \mathbb Z.
• References should go in the master reference file doc/en/reference/references/index.rst and have more descriptive names.

• Format like:

  def foo(self):
    """
    Short description.
  
    ...
    """

• Your ascii art square for ConvexCore should be is of the form:: (with a double colon). Although having spaces here will be better than the .. The doc should not have any trouble getting this to look nice.
• There are a number of variables that should be either formatted for latex (`x`) or code (``x``). In particular, things like ``False`` should be code formatted.
• It is better to put inputs for __init__ in the class-level docstring so it will be visible.

Some code issue I noticed on a quick readthrough:

• It would be good to provide some sort of deprecation warnings for all non-hidden functions/classes (i.e., not _lexi_gen). See the deprecation option of lazy_import.
• Use new-style Python clases: class ConvexCore(object):. Although can the user ever see it? If so, perhaps it should inherit from SageObject.
• Never check things like if x is True: (resp. False:); simply do if x: (resp. if not x:).
• It is faster to check if todo: instead of if len(todo) > 0.
• Instead of __str__, if it is a SageObject subclass, you should use _repr_ (e.g., subclasses of DiGraph). Otherwise, we prefer to use __repr__.
• groups/free_groups/test_train_track.py might be better as tests/train_tracks.py.

[tscrim]

comment:109

However, I will start going through the code part soon.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

573701b

some correction from tscrim comments

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from b81e62d to 573701b

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 573701b to dcc540c

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

dcc540c

some correction from tscrim comments