#6569: sparse integer matrix doesn't raise an error on non-integer index
------------------------------+---------------------------------------------
Reporter: jason | Owner: was
Type: defect | Status: positive_review
Priority: major | Milestone: sage-5.0
Component: linear algebra | Keywords:
Work_issues: | Upstream: N/A
Reviewer: | Author: Michael Orlitzky
Merged: | Dependencies:
------------------------------+---------------------------------------------
Changes (by was):
* status: needs_review => positive_review
Comment:
1. I just looked at this and I'm shocked that {{{ind}}} is of type int.
This will work fine on 32-bit, but will be totally broken/buggy in subtle
and surprising ways on 64-bit machines. The type of ind should be
{{{Py_ssize_t}}}, just like the type of {{{i}}}.
I realize that your patch does not introduce ind. It was introduced in
#4973...
Nobody will notice this now, because there is some sort of massive weird
inefficiency in the creation of sparse matrices (maybe the parent space
has its basis constructed or something idiotic like that), which makes it
impossible to make a large sparse matrix, even though this should be
trivial, instant, fast, and take no memory:
{{{
sage: time a = matrix(QQ, 2^25, sparse=True)
Time: CPU 5.88 s, Wall: 8.46 s
sage: get_memory_usage() # what ?
3908.29296875
}}}
I've made the ind and memory issue trac #12348.
2. That said, everything in this particular patch looks great to me.
Positive review!
I hope you can address #12348 though next. At least the trivial change of
ind to {{{Py_ssize_t}}}.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/6569#comment:2>
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.
