#10628: initialization of matrices from vectors or list of lists can be way
faster
------------------------------+---------------------------------------------
Reporter: mderickx | Owner: jason, was
Type: enhancement | Status: needs_work
Priority: minor | Milestone:
Component: linear algebra | Keywords:
Work_issues: | Upstream: N/A
Reviewer: | Author:
Merged: | Dependencies:
------------------------------+---------------------------------------------
Changes (by SimonKing):
* status: needs_review => needs_work
Comment:
If you look at the init method of `Matrix_generic_dense`, you see that the
given list of entries is simply assigned to the attribute `self._entries`,
if copy is False, and a copy of the list of entries is assigned to
`self._entries` otherwise.
Note that the init method of `Matrix_generic_dense` actually does not use
the "copy" function: The copy is constructed explicitly. I could imagine
that this is not optimal, but that's a different topic.
Now, is it safe to keep "copy=False" in your patch? I think so. You create
a new list, both when you transform a list of vectors resp. a list of
lists into a single list of entries. So, "copy=True" would be a waste of
time.
Concerning the test for the presence of "is_vector": I think one could
easily do better.
Whithout your patch, what one actually used was the method "list()" of
vectors (see line 28 of your patch). With your patch, you use that a
vector is iterable, by doing e.extend(v) in line 30 of your patch.
If I am not mistaken, good Python tradition is to duck test using the
methods that are actually needed. Hence, without your patch, it would have
been better to simply try to use "v.list()", and catch the attribute
error. And with your patch, it would be better to simply try
"e.extend(v)", and catch the error that results if v is not iterable.
Moreover, I really think that the following should be made work:
{{{
sage: MS = MatrixSpace(GF(5),4,2)
sage: MS0 = MatrixSpace(GF(5),2)
sage: MS([MS0.random_element(), MS0.random_element()])
}}}
Hence, the matrix constructor should also accept lists of matrices (not
just list of vectors), and should automatically stack the matrices from
the list.
The problem with duck typing via "list()" is that we could have matrices
over a ring whose elements have a list() method. In that case, we would
not want to use list() in the matrix constructor.
Perhaps one could proceed as follows:
{{{
#!python
if isinstance(x, list) and len(x) > 0:
# If the list has the expected length, then
# we are likely to have a list of elements of
# the base ring
if len(x) < self.__ncols*self.__nrows:
e = []
for v in x:
try:
e.extend(v.list())
except AttributeError:
e.extend(v)
x = e
# x is a new list, hence, copy=False is OK
copy = False
# Now, x presumably is a list of ring elements.
if not rows:
new_x = []
for k in xrange(len(x)):
i = k % self.__ncols
j = k // self.__ncols
new_x.append( x[ i*self.__nrows + j ] )
x = new_x
return self.__matrix_class(self, entries=x, copy=copy,
coerce=coerce)
}}}
Do you think that my idea makes sense? It would allow to define a matrix
by a list of (lists, matrices, vectors) -- the three could actually be
combined.
I have to leave office now, but I hope that I'll find time to create a
patch later or tomorrow.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10628#comment:9>
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.
