octave-bug-tracker
[Top][All Lists]
Advanced

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

[Octave-bug-tracker] [bug #53198] nnz could be improved for diagonal mat


From: Dan Sebald
Subject: [Octave-bug-tracker] [bug #53198] nnz could be improved for diagonal matrices
Date: Wed, 21 Feb 2018 15:01:14 -0500 (EST)
User-agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

Follow-up Comment #2, bug #53198 (project octave):

I've looked at the patch.  The conditional test

if (is_diag_matrix ())

in the patch appears to be superfluous.  Isn't the class, by definition,
diagonal?  Wouldn't simply extracting the diagonal, i.e., collapsing to a
vector, be sufficient?  I.e.,


  octave_idx_type nnz (void) const 
  {
    return diag ().nnz ();
  }


However, there is some special code in the ::diag() method:


  if (matrix.rows () == 1 || matrix.cols () == 1)
    {
      // Rather odd special case.  This is a row or column vector
      // represented as a diagonal matrix with a single nonzero entry, but
      // Fdiag semantics are to product a diagonal matrix for vector
      // inputs.
      if (k == 0)
        // Returns Diag2Array<T> with nnz <= 1.
        retval = matrix.build_diag_matrix ();
      else
        // Returns Array<T> matrix
        retval = matrix.array_value ().diag (k);
    }
  else
    // Returns Array<T> vector
    retval = matrix.extract_diag (k);
  return retval;


Is that special case something to be concerned about?  I think that would
arise for the following sort of construct:


octave:17> a = diag(1,1,10000);
octave:18> isdiag(a)
ans = 1
octave:19> size(a)
ans =

       1   10000

octave:20> size(diag(a,0))
ans =

   10000   10000



(Note how the application of diag() with k=0, the default in calling the
::diag() member function, expands the matrix in this case, not contracts it.)

Also, throughout ov-base-diag.h are several to_dense() calls.  Maybe they all
could be addressed with one patch here.  For example, there is the method
"sort" that uses to_dense() first, but that too could be improved speed-wise
noting that the operation boils down to pushing the diagonal value either to
the first or last row, WLOG, e.g.,


octave:10> sort(diag([1 -2 3]))
ans =

   0  -2   0
   0   0   0
   1   0   3


(if it is sort-by-row, then change the rule for the other direction).  One
would just start with the appropriately sized zeros matrix and then index
through the diagonal vector sending the values to the top or bottom row based
on one less-than-zero comparison.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?53198>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/




reply via email to

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