#11422: modular subgroups
-----------------------------------------+----------------------------------
Reporter: vdelecroix | Owner: vdelecroix
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.7.1
Component: modular forms | Keywords: group, arithmetic,
linear group, sl, modular
Work_issues: corrections to docstrings | Upstream: N/A
Reviewer: | Author: Vincent Delecroix
Merged: | Dependencies:
-----------------------------------------+----------------------------------
Changes (by davidloeffler):
* status: needs_review => needs_work
* work_issues: => corrections to docstrings
Comment:
OK, I downloaded #10335 and its prerequisites and it works fine now.
Algorithm comments:
- For permutation subgroups, almost all calculations are done with a
relabelled version, but this is generated anew every time! I'd suggest:
- having a flag that remembers whether the group is already canonically
labelled,
- when that's not the case, caching the canonically labelled version.
- At line 1019 of {{{arithgroup.py}}}, if check is True, won't this cause
the check code to be run *twice*?
Corrections to docstrings:
In {{{arithgroup_generic.py}}}:
- The docstring for the todd_coxeter method is a bit vague: I'd suggest
that you specify a bit more precisely what the lists {{{l}}} and {{{s}}}
mean. "The action of the parabolic elements" is misleading; you mean the
action of the specific parabolic element {{{ [[1, 1],[0,1]] }}}.
- It is disrespectful to Todd and Coxeter to write their names in
lowercase, as you do in the docstring for {{{as_permutation_group}}} --
better to write "This uses Todd-Coxeter enumeration".
In {{{arithgroup_perm.py}}}:
- "point of vue" --> "point of view" in the module docstring (as I pointed
out above)
- Again in that docstring, you make it sound like all four of L, R, S2 and
S3 are needed to generate the modular group -- explain that the "three
couples which are of main interest" are of interest because each pair of
elements generates the whole group.
- The "todo" list will look bizzarre: some LaTeX formatting is needed for
the second item
- Line 328: "generator" --> "generators"
- Line 630 "numerotation" --> "numbering"
- 640: "defines" --> "define", "renumerote" --> "renumber" (the
nonexistent word "numerote" appears twice more in the next few lines)
- 744: "Returns the smallest label among rooted label" -- do you seriously
expect that anyone will have the foggiest clue what this means?
- 749: "canonic" --> "canonical". This occurs all over the place. Fix them
all.
- 810: needs more explanation of what the argument j0 does.
- 812: "renumerotation" --> "renumbering" again.
- 908: "does not permutes" --> "does not permute"
- 947: "at the first times" --> "at the first time"
That's as far as I've got so far -- other duties call.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11422#comment:8>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
