octave-maintainers
[Top][All Lists]
Advanced

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

the purpose of Sparse<T>::nzmax


From: John W. Eaton
Subject: the purpose of Sparse<T>::nzmax
Date: Fri, 9 Apr 2010 09:46:41 -0400

On  9-Apr-2010, Jaroslav Hajek wrote:

| I'm wondering about the purpose of Sparse<T>::nzmax. The comment in
| the source says that it's supposed to be an alias for capacity(),
| which should be the amount of storage allocated for nonzero elements,
| whereas nnz() is the actual amount of nonzero elements (equivalent to
| cidx(cols())).
| 
| However, it seems that contrary to its "documentation", nzmax is
| generally used throughout both liboctave and interpreter as a synonym
| for nnz. I believe this may be the reason behind bug #29483. Until my
| recent changes, nnz() and capacity() were probably always equal, but
| now that Sparse matrix tries to avoid some reallocations (mainly, if
| maybe_compress discards just a few elements, the arrays are *not*
| reallocated and copied to truncate the excess storage), this invariant
| no longer holds.
| 
| For instance, consider the code
| 
| SparseBoolMatrix::operator ! (void) const
| {
|   octave_idx_type nr = rows ();
|   octave_idx_type nc = cols ();
|   octave_idx_type nz1 = nzmax ();  # GOTCHA
|   octave_idx_type nz2 = nr*nc - nz1;
| 
|   SparseBoolMatrix r (nr, nc, nz2);
| 
|   octave_idx_type ii = 0;
|   octave_idx_type jj = 0;
|   r.cidx (0) = 0;
|   for (octave_idx_type i = 0; i < nc; i++)
|     {
|       for (octave_idx_type j = 0; j < nr; j++)
|         {
|           if (jj < cidx(i+1) && ridx(jj) == j)
|             jj++;
|           else
|             {
|               r.data(ii) = true;
|               r.ridx(ii++) = j;
|             }
|         }
|       r.cidx (i+1) = ii;
|     }
| 
|   return r;
| }
| 
| It's all good except the GOTCHA, because obviously it is the true
| number of nonzero elements that matters, not the amount of storage.
| 
| This particular line is due to John's 5604:2857357f9d3c, but that
| seems to be just some renaming changeset, so it appears this stuff is
| even older.
| What do you think is the best solution? Shall I try to find invalid
| nzmax usages, or can nzmax be made an alias of nnz() (which may
| possibly break something else)?

David will probably know more, but my understanding is that nzmax was
originally present for compatibility with Matlab, but not really used
internally because Octave's sparse array type never allocated more
storage than necessary.  Sorry if my change back then screwed things
up.  I'm not sure what was intended there, other than to be
consistent, and maybe that was wrong.

If nnz and nzmax are not always the same now, then I think we need to
use each appropriately.  We should also update the nzmax docstring in
src/data.cc so that it doesn't lead people to think that nnz and nzmax
are usually interchangeable in Octave.

I see that we have duplicate code for the following functions where I
think it could be in a single template function:

  SparseMatrix::operator ! (void) const
  SparseBoolMatrix::operator ! (void) const
  SparseComplexMatrix::operator ! (void) const

and only the SparseBoolMatrix version of this function has

  octave_idx_type nz1 = nzmax ();

while the others use nnz ().

jwe


reply via email to

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