#9562: Add M4RIE to Sage
---------------------------------------------+------------------------------
Reporter: malb | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-5.0
Component: packages | Keywords: m4ri
Work_issues: Improve scalar multiplication | Upstream: N/A
Reviewer: Paul Zimmermann, Simon King | Author: Martin Albrecht
Merged: | Dependencies: #11574
---------------------------------------------+------------------------------
Changes (by SimonKing):
* status: needs_review => needs_work
Comment:
Finally, the tests on 32 bit solaris (mark on skynet) are finshed. They
passed, modulo the usual timeouts. The long tests pass on my machine. If I
understand the discussion above, it builds on 64 bit solaris and on Cygwin
as well, and of course on linux and os x.
Moreover, it provides a very impressive speedup compared with old Sage
matrices over GF(2^e^). If I am not mistaken, it is even faster than
Magma.
So, it is almost a positive review.
But there remain a few things to do.
1. David Kirkby noted that we are adding a new standard spkg here. So,
there should be a voting on sage-devel. So, that's "needs info".
2. I found some issues with the "randomize" methods.
i) The randomize method in sage/libs/ntl/ntl_mat_GF2E.pyx is not
documented (thus, also has no tests), and it lacks the usual optional
arguments `density` and `nonzero`.
ii) The doc of Matrix_mod2e_dense.randomize gives no use cases for the
optional arguments. Actually, the behaviour when passing the optional
arguments is clearly not what we want:
{{{
sage: MS = MatrixSpace(GF(64,'a'),100,100)
sage: A = MS.random_element(densitiy=0.3)
sage: A.density()
4913/5000
sage: B = MS.random_element(nonzero=True)
sage: B.density()
2469/2500
sage: C = MS.random_element(nonzero=True, density=1)
sage: C.density()
2461/2500
sage: D = MS.random_element(density=0)
sage: D.density()
983/1000
}}}
3. The doc of many methods needs some polishing.
* It would be nice to have a reference to research articles. I had never
heard of Travolta tables before. Strassen-Winograd and Karatsuba may be
better known, but still, a reference could help.
* _matrix_times_matrix is not documented.
* `__init__`, `__cinit__` do not specify their arguments.
* `_lmul_`, `__neg__`, `__richcmp__`, `__invert__`, `__reduce__`,
`set_unsafe` and `get_unsafe` do not state what they do, and the method
names do not occur in the example (so, it should be marked as an indirect
doctest).
* cdef'd methods such as rescale_row_c or add_multiple_of_row_c or
swap_rows_c and so on may not be as easily visible by the user than Python
methods. However, as a courtesy to developers who actually read the source
file, the arguments of those methods should be specified.
* "echelonize" should state what it does and what its optional arguments
are. If I am not mistaken, it changes the matrix inplace, and that should
be documented.
* When I read "Classical cubic matrix multiplication.", I first
understood that the matrix is cubic. But perhaps I'm a bit square here...
So, that's "needs work" for now.
Since there should be a vote on sage-devel anyway, we might also ask
whether it should be included in the docs. I am still not sure: On the one
hand, I think a reference manual should be thorough. On the other hand,
all "new" methods that are no cdef'd methods and do not start with an
underscore overwrite methods from super-classes that are documented
elsewhere.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9562#comment:88>
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.
