[SciPy-dev] Possible fix for scipy.sparse.lil_matrix column-slicing problem

[Tim Victor]

On Tue, Dec 1, 2009 at 11:13 AM, Robert Cimrman wrote:
> Tim Victor wrote:
>> On Tue, Dec 1, 2009 at 8:11 AM, Robert Cimrman wrote:
>>> I think scipy.sparse indexing should follow the behavior of numpy dense arrays.
>>>  This is what current SVN scipy does (0.8.0.dev6122):
>>>
>>> In [1]: import scipy.sparse as spp
>>> In [2]: a = spp.lil_matrix((10,10))
>>> In [3]: a[range(10),0] = 1
>>>
>>> This is ok.
>>>
>>> In [5]: a[range(10),range(10)] = -1
>>> In [8]: print a.todense()
>>> ------> print(a.todense())
>>> [[-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]
>>>  [-1. -1. -1. -1. -1. -1. -1. -1. -1. -1.]]
>>>
>>> This is IMHO not ok (what other sparse matrix users think?)
>>>
>>> In [9]: import scipy as sp
>>> In [10]: a[range(10),range(9,-1,-1)] = sp.rand(10)
>>> In [12]: print a.todense()
>  >> <snip>
>>>
>>> same as above...
>>>
>>>> I'm reminded of the Zen of Python "Explicit is better than implicit." guideline.
>>> :)
>>>
>>> Consider also this: the current behavior (broadcasting the index arrays to the
>>> whole rectangle) is not compatible with NumPy, and does not allow setting
>>> elements e.g. in a random sequence of positions. On the other hand, the
>>> broadcasting behaviour can be easily, explicitely, obtained by using
>>> numpy.mgrid and similar functions.
>>>
>>>> Best regards, and many apologies for the inconvenience,
>>> No problem, any help and code contribution is more than welcome!
>>>
>>> I guess that fixing this issue should not be too difficult, so you could make
>>> another stab :-) If there is a consensus here, of course... (Nathan?)
>>>
>>> cheers,
>>> r.
>>
>> Yes, I agree with you 100%, Robert. The behavior of NumPy for dense
>> arrays should be the guide, and I tried to follow it but didn't know
>> to check that case.
>
> No problem at all. It was a coincidence that I stumbled on a case that was not
> covered by the tests. I do not even uses lil_matrix much :)
>
>> I don't defend how my version handles your case where the i and j
>> indexes are both sequences. The behavior that you expect is correct
>> and I plan to fix it to make your code work. I would however like to
>> make sure that I understand it well and get it all correct this
>> time--including correctly handling the case where the right-hand side
>> is also a sequence.
>
> Sure! I am not an expert in this either, so let's wait a bit if somebody chimes
> in... Could you summarize this discussion in a new ticket, if you have a little
> spare time?
>
> Note that I do not push this to be fixed soon by any means, my code already
> runs ok with the current version. So take my "bugreport" easy ;)
>
> Best,
> r.

Finally had some time to figure out the ticket system and create one!

http://projects.scipy.org/scipy/ticket/1075

I hope I did that right.

My teaching duties are over for the semester and I should have some
time to dive back into the code from now until the end of the year.

Best regards,

Tim Victor