[Octave-bug-tracker] [bug #51880] sparse() ignores value of scalar column index input arg

[Rik]

Follow-up Comment #6, bug #51880 (project octave):

Perhaps we should step away from this and look at it a different way.  Why
does the constructor need to create an empty SparseRep and then use
change_capacity to increase it?  I understand from a code clarity
point-of-view that it guarantees a valid object is returned by the function
regardless of which twists and turns the code takes through the various
if/elseif branches.  However, given that this is a base library, I think it is
more important to emphasize performance.

If we wait in the Sparse constructor until we know the number of elements,
then we can use the this SparseRep constructor


    SparseRep (octave_idx_type nr, octave_idx_type nc, octave_idx_type nz =
0)
      : d (nz > 0 ? new T [nz] {} : nullptr),
        r (nz > 0 ? new octave_idx_type [nz] {} : nullptr),
        c (new octave_idx_type [nc+1] {}), nzmx (nz), nrows (nr),
        ncols (nc), count (1)
    { }


Because of the empty initializer list, the variables d, r, and c will all be
filled with zeros.  That means that only the cidx entries from c(0) to nc need
to be filled in rather than running over all columns nc.

Instead of the newly changed code from jwe:


  xcidx (0) = 0;
  for (octave_idx_type j = 0; j < nc; j++)
    xcidx (j+1) = j >= c(0) ? new_nz : 0;


It could be


// xcidx (0) = 0;  # no longer necessary, cidx is initialzed to all 0
std::fill_n (xcidx () + c(0) + 1, nc - c(0), new_nz);


This would also avoid some for loops and potential new[] operators in
change_capacity.



    _______________________________________________________

Reply to this item at:

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

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