octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Pkg-octave-devel] Bug#706376: Bug#706376: Bug#706376: Bug#706376: o


From: David Bateman
Subject: Re: [Pkg-octave-devel] Bug#706376: Bug#706376: Bug#706376: Bug#706376: octave: sparse matrix n*2^16
Date: Wed, 19 Jun 2013 20:06:14 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6

On 06/16/2013 03:59 AM, Jordi Gutiérrez Hermoso wrote:
> I'm saying the real problem is that we assume linear indexing works
> for all matrix types, including sparse matrices. I claim that this is
> the real problem. 
Who is assuming linear indexing works for all matrix types ? Where
exactly is that stated ? If its in then manual that its a bug in the
manual as linear indexing can never work correctly in this case because
the underlying 32 integer type won't allowed it and in fact octave as it
stand throws an error !!!

> We can patch around this problem by avoiding linear
> indexing,

The bug report was in "trace" that called "isempty" on a sparse matrix.
Neither function needs "numel" or "linear indexing". We aren't patching
around anything, we are fixing a bug

>  but this is just treating the symptoms, not the disease.

So you advocate everyone moving to 64-bit indexing ? In that case what
happens when we get the same bug report for s(2^32, 2^32) ? Is that a
"disease" too.  Certainly there is a limitation on linear indexing and
numel for sparse matrices. We should document it and make as many things
as possible work with sparse matrices and be done with it.

> While I don't deny that we can make some progress masking the
> symptoms, the disease itself should also be treated somehow.

There is no disease, and unless you want to artificially limit the size
of sparse matrices that can be treated such that numel is less than 2^31
for 32 bit indexing and 2^63 for 64 bit indexing. Why do this which
makes sparse matrices much less useful, so there is no solution for what
you call a disease

>> So essentially you're saying that sparse matrices with
>> 32-bit indexing and numel larger than 2^31 are useless!!
> I'm saying that they will fail in other unexpected ways,

Isn't that the definition of a bug.

>  and we shouln't mask symptoms. 

We never tried to. Look at the code in dim-vector.h

<quote>
  // The following function will throw a std::bad_alloc ()
  // exception if the requested size is larger than can be indexed by
  // octave_idx_type. This may be smaller than the actual amount of
  // memory that can be safely allocated on a system.  However, if we
  // don't fail here, we can end up with a mysterious crash inside a
  // function that is iterating over an array using octave_idx_type
  // indices.

  octave_idx_type safe_numel (void) const;
</quote>

The numel method of Sparse<T> calls this method that is supposed to
throw an error. However as the builtin Fnumel is calling args(1).numel()
which is calling dims().numel() the sparse safe version of numel isn't
being called. The solution is to add a numel method to
octave_base_sparse that calls dims().safe_numel() instead. So this is a
bug as well.

As for linear indexing, if you look in idx-vector.cc you'll see that in
the convert_index functions an error is returned like

octave_idx_type idx = static_cast<octave_idx_type>(d)
bool err = static_cast<double> (idx) != d;

So as expected

s = speye (2^17);
s (2^32)

throws an error

error: subscript indices must be either positive integers or logicals

You might think this is a little cryptic but I wouldn't call it "masking
a symptom". I propose modifying this error to read

error: subscript indices must be either positive integers less than 2^31
or logicals

for 32 bit indexing and

error: subscript indices must be either positive integers less than 2^63
or logicals

for 64 bit indexing. See the attached changeset

David



Attachment: patch.is_empty2
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]