# HG changeset patch # User John W. Eaton # Date 1627605793 14400 # Node ID 6a8642d310c851792778a51120c162d2a7c07df5 # Parent 6a39ac893c9da6946e12ee36f4a7e787c9bb1c94 style fixes for Sparse class * Sparse.h, Sparse.cc: (SparseRep::m_data): Rename from m_d. Change all uses. (SparseRep::m_ridx) Rename form m_r. Change all uses. (SparseRep::m_cidx): Rename from m_c. Change all uses. (SparseRep::m_nzmax): Rename from m_nzmx. Change all uses. Change other instances of nzmx to be nzmax. (SparseRep::elem, SparseRep::celem): Rename function parameters to r and c from _r and _c. (SparseRep::operator = (const Sparse_rep&)): Tag as deleted instead of defining as private. (SparseRep::nzmax): New function. Change all uses of length to mzmax and comment that nzmax is to be preferred over length. (SparseRep::rows, SparseRep::cols, SparseRep::columns): New functions. (class Sparse): Make data members protected instead of public. (SparseRep): Declare Sparse class as friend. (SparseRep): New constructors that accept data elements directly. (Sparse::Sparse (const Sparse&)): Simplify using new SparseRep constructor. diff -r 6a39ac893c9d -r 6a8642d310c8 liboctave/array/Sparse.cc --- a/liboctave/array/Sparse.cc Wed Jul 28 07:19:31 2021 -0400 +++ b/liboctave/array/Sparse.cc Thu Jul 29 20:43:13 2021 -0400 @@ -64,54 +64,54 @@ template OCTAVE_API T& -Sparse::SparseRep::elem (octave_idx_type _r, octave_idx_type _c) +Sparse::SparseRep::elem (octave_idx_type r, octave_idx_type c) { octave_idx_type i; - if (m_nzmx <= 0) + if (m_nzmax <= 0) (*current_liboctave_error_handler) ("Sparse::SparseRep::elem (octave_idx_type, octave_idx_type): sparse matrix filled"); - for (i = m_c[_c]; i < m_c[_c + 1]; i++) - if (m_r[i] == _r) - return m_d[i]; - else if (m_r[i] > _r) + for (i = m_cidx[c]; i < m_cidx[c + 1]; i++) + if (m_ridx[i] == r) + return m_data[i]; + else if (m_ridx[i] > r) break; // Ok, If we've gotten here, we're in trouble. Have to create a // new element in the sparse array. This' gonna be slow!!! - if (m_c[m_ncols] == m_nzmx) + if (m_cidx[m_ncols] == m_nzmax) (*current_liboctave_error_handler) ("Sparse::SparseRep::elem (octave_idx_type, octave_idx_type): sparse matrix filled"); - octave_idx_type to_move = m_c[m_ncols] - i; + octave_idx_type to_move = m_cidx[m_ncols] - i; if (to_move != 0) { - for (octave_idx_type j = m_c[m_ncols]; j > i; j--) + for (octave_idx_type j = m_cidx[m_ncols]; j > i; j--) { - m_d[j] = m_d[j-1]; - m_r[j] = m_r[j-1]; + m_data[j] = m_data[j-1]; + m_ridx[j] = m_ridx[j-1]; } } - for (octave_idx_type j = _c + 1; j < m_ncols + 1; j++) - m_c[j] = m_c[j] + 1; - - m_d[i] = 0.; - m_r[i] = _r; - - return m_d[i]; + for (octave_idx_type j = c + 1; j < m_ncols + 1; j++) + m_cidx[j] = m_cidx[j] + 1; + + m_data[i] = 0.; + m_ridx[i] = r; + + return m_data[i]; } template OCTAVE_API T -Sparse::SparseRep::celem (octave_idx_type _r, octave_idx_type _c) const +Sparse::SparseRep::celem (octave_idx_type r, octave_idx_type c) const { - if (m_nzmx > 0) - for (octave_idx_type i = m_c[_c]; i < m_c[_c + 1]; i++) - if (m_r[i] == _r) - return m_d[i]; + if (m_nzmax > 0) + for (octave_idx_type i = m_cidx[c]; i < m_cidx[c + 1]; i++) + if (m_ridx[i] == r) + return m_data[i]; return T (); } @@ -126,18 +126,18 @@ octave_idx_type k = 0; for (octave_idx_type j = 1; j <= m_ncols; j++) { - octave_idx_type u = m_c[j]; + octave_idx_type u = m_cidx[j]; for (; i < u; i++) - if (m_d[i] != T ()) + if (m_data[i] != T ()) { - m_d[k] = m_d[i]; - m_r[k++] = m_r[i]; + m_data[k] = m_data[i]; + m_ridx[k++] = m_ridx[i]; } - m_c[j] = k; + m_cidx[j] = k; } } - change_length (m_c[m_ncols]); + change_length (m_cidx[m_ncols]); } template @@ -145,32 +145,32 @@ void Sparse::SparseRep::change_length (octave_idx_type nz) { - for (octave_idx_type j = m_ncols; j > 0 && m_c[j] > nz; j--) - m_c[j] = nz; + for (octave_idx_type j = m_ncols; j > 0 && m_cidx[j] > nz; j--) + m_cidx[j] = nz; // Always preserve space for 1 element. nz = (nz > 0 ? nz : 1); // Skip reallocation if we have less than 1/frac extra elements to discard. static const int frac = 5; - if (nz > m_nzmx || nz < m_nzmx - m_nzmx/frac) + if (nz > m_nzmax || nz < m_nzmax - m_nzmax/frac) { // Reallocate. - octave_idx_type min_nzmx = std::min (nz, m_nzmx); + octave_idx_type min_nzmax = std::min (nz, m_nzmax); octave_idx_type *new_ridx = new octave_idx_type [nz]; - std::copy_n (m_r, min_nzmx, new_ridx); - - delete [] m_r; - m_r = new_ridx; + std::copy_n (m_ridx, min_nzmax, new_ridx); + + delete [] m_ridx; + m_ridx = new_ridx; T *new_data = new T [nz]; - std::copy_n (m_d, min_nzmx, new_data); - - delete [] m_d; - m_d = new_data; - - m_nzmx = nz; + std::copy_n (m_data, min_nzmax, new_data); + + delete [] m_data; + m_data = new_data; + + m_nzmax = nz; } } @@ -179,7 +179,7 @@ bool Sparse::SparseRep::indices_ok (void) const { - return sparse_indices_ok (m_r, m_c, m_nrows, m_ncols, nnz ()); + return sparse_indices_ok (m_ridx, m_cidx, m_nrows, m_ncols, nnz ()); } template @@ -190,7 +190,7 @@ octave_idx_type nz = nnz (); for (octave_idx_type i = 0; i < nz; i++) - if (octave::math::isnan (m_d[i])) + if (octave::math::isnan (m_data[i])) return true; return false; @@ -273,13 +273,13 @@ ("Sparse::Sparse (const Sparse&, const dim_vector&): dimension mismatch"); dim_vector old_dims = a.dims (); - octave_idx_type new_nzmx = a.nnz (); + octave_idx_type new_nzmax = a.nnz (); octave_idx_type new_nr = dv(0); octave_idx_type new_nc = dv(1); octave_idx_type old_nr = old_dims(0); octave_idx_type old_nc = old_dims(1); - m_rep = new typename Sparse::SparseRep (new_nr, new_nc, new_nzmx); + m_rep = new typename Sparse::SparseRep (new_nr, new_nc, new_nzmax); octave_idx_type kk = 0; xcidx (0) = 0; @@ -296,7 +296,7 @@ xridx (j) = ii; } for (octave_idx_type k = kk; k < new_nc; k++) - xcidx (k+1) = new_nzmx; + xcidx (k+1) = new_nzmax; } template @@ -679,14 +679,14 @@ octave_idx_type nr = rows (); octave_idx_type nc = cols (); octave_idx_type len = a.numel (); - octave_idx_type new_nzmx = 0; + octave_idx_type new_nzmax = 0; // First count the number of nonzero terms for (octave_idx_type i = 0; i < len; i++) if (a(i) != T ()) - new_nzmx++; - - m_rep = new typename Sparse::SparseRep (nr, nc, new_nzmx); + new_nzmax++; + + m_rep = new typename Sparse::SparseRep (nr, nc, new_nzmax); octave_idx_type ii = 0; xcidx (0) = 0; @@ -1019,13 +1019,13 @@ if (c != m_rep->m_ncols) { octave_idx_type *new_cidx = new octave_idx_type [c+1]; - std::copy_n (m_rep->m_c, std::min (c, m_rep->m_ncols) + 1, new_cidx); - delete [] m_rep->m_c; - m_rep->m_c = new_cidx; + std::copy_n (m_rep->m_cidx, std::min (c, m_rep->m_ncols) + 1, new_cidx); + delete [] m_rep->m_cidx; + m_rep->m_cidx = new_cidx; if (c > m_rep->m_ncols) - std::fill_n (m_rep->m_c + m_rep->m_ncols + 1, c - m_rep->m_ncols, - m_rep->m_c[m_rep->m_ncols]); + std::fill_n (m_rep->m_cidx + m_rep->m_ncols + 1, c - m_rep->m_ncols, + m_rep->m_cidx[m_rep->m_ncols]); } m_rep->m_ncols = m_dimensions(1) = c; @@ -3082,12 +3082,12 @@ Sparse::print_info (std::ostream& os, const std::string& prefix) const { os << prefix << "m_rep address: " << m_rep << "\n" - << prefix << "m_rep->m_nzmx: " << m_rep->m_nzmx << "\n" + << prefix << "m_rep->m_nzmax: " << m_rep->m_nzmax << "\n" << prefix << "m_rep->m_nrows: " << m_rep->m_nrows << "\n" << prefix << "m_rep->m_ncols: " << m_rep->m_ncols << "\n" - << prefix << "m_rep->data: " << static_cast (m_rep->m_d) << "\n" - << prefix << "m_rep->ridx: " << static_cast (m_rep->m_r) << "\n" - << prefix << "m_rep->cidx: " << static_cast (m_rep->m_c) << "\n" + << prefix << "m_rep->m_data: " << m_rep->m_data << "\n" + << prefix << "m_rep->m_ridx: " << m_rep->m_ridx << "\n" + << prefix << "m_rep->m_cidx: " << m_rep->m_cidx << "\n" << prefix << "m_rep->m_count: " << m_rep->m_count << "\n"; } diff -r 6a39ac893c9d -r 6a8642d310c8 liboctave/array/Sparse.h --- a/liboctave/array/Sparse.h Wed Jul 28 07:19:31 2021 -0400 +++ b/liboctave/array/Sparse.h Thu Jul 29 20:43:13 2021 -0400 @@ -61,65 +61,93 @@ { public: - T *m_d; - octave_idx_type *m_r; - octave_idx_type *m_c; - octave_idx_type m_nzmx; + T *m_data; + octave_idx_type *m_ridx; + octave_idx_type *m_cidx; + octave_idx_type m_nzmax; octave_idx_type m_nrows; octave_idx_type m_ncols; octave::refcount m_count; SparseRep (void) - : m_d (new T [1]), m_r (new octave_idx_type [1] {}), - m_c (new octave_idx_type [1] {}), - m_nzmx (1), m_nrows (0), m_ncols (0), m_count (1) + : m_data (new T [1]), m_ridx (new octave_idx_type [1] {}), + m_cidx (new octave_idx_type [1] {}), + m_nzmax (1), m_nrows (0), m_ncols (0), m_count (1) { } SparseRep (octave_idx_type n) - : m_d (new T [1]), m_r (new octave_idx_type [1] {}), - m_c (new octave_idx_type [n+1] {}), - m_nzmx (1), m_nrows (n), m_ncols (n), m_count (1) + : m_data (new T [1]), m_ridx (new octave_idx_type [1] {}), + m_cidx (new octave_idx_type [n+1] {}), + m_nzmax (1), m_nrows (n), m_ncols (n), m_count (1) { } SparseRep (octave_idx_type nr, octave_idx_type nc, octave_idx_type nz = 1) - : m_d (nz > 0 ? new T [nz] : new T [1]), - m_r (nz > 0 ? new octave_idx_type [nz] {} : new octave_idx_type [1] {}), - m_c (new octave_idx_type [nc+1] {}), - m_nzmx (nz > 0 ? nz : 1), m_nrows (nr), m_ncols (nc), m_count (1) + : m_data (nz > 0 ? new T [nz] : new T [1]), + m_ridx (nz > 0 ? new octave_idx_type [nz] {} : new octave_idx_type [1] {}), + m_cidx (new octave_idx_type [nc+1] {}), + m_nzmax (nz > 0 ? nz : 1), m_nrows (nr), m_ncols (nc), m_count (1) { } - SparseRep (const SparseRep& a) - : m_d (new T [a.m_nzmx]), m_r (new octave_idx_type [a.m_nzmx]), - m_c (new octave_idx_type [a.m_ncols + 1]), - m_nzmx (a.m_nzmx), m_nrows (a.m_nrows), m_ncols (a.m_ncols), m_count (1) + SparseRep (octave_idx_type nr, octave_idx_type nc, octave_idx_type nz, + const T *d, const octave_idx_type *r, const octave_idx_type *c) + : m_data (new T [nz]), + m_ridx (new octave_idx_type [nz] {}), + m_cidx (new octave_idx_type [nc+1] {}), + m_nzmax (nz), m_nrows (nr), m_ncols (nc), m_count (1) { - octave_idx_type nz = a.nnz (); - std::copy_n (a.m_d, nz, m_d); - std::copy_n (a.m_r, nz, m_r); - std::copy_n (a.m_c, m_ncols + 1, m_c); + std::copy_n (d, nz, m_data); + std::copy_n (r, nz, m_ridx); + std::copy_n (c, m_ncols + 1, m_cidx); + } + + template + SparseRep (octave_idx_type nr, octave_idx_type nc, octave_idx_type nz, + const U *d, const octave_idx_type *r, const octave_idx_type *c) + : m_data (new T [nz]), + m_ridx (new octave_idx_type [nz] {}), + m_cidx (new octave_idx_type [nc+1] {}), + m_nzmax (nz), m_nrows (nr), m_ncols (nc), m_count (1) + { + std::copy_n (d, nz, m_data); + std::copy_n (r, nz, m_ridx); + std::copy_n (c, nc + 1, m_cidx); } - ~SparseRep (void) { delete [] m_d; delete [] m_r; delete [] m_c; } - - octave_idx_type length (void) const { return m_nzmx; } + SparseRep (const SparseRep& a) + : m_data (new T [a.m_nzmax]), m_ridx (new octave_idx_type [a.m_nzmax]), + m_cidx (new octave_idx_type [a.m_ncols + 1]), + m_nzmax (a.m_nzmax), m_nrows (a.m_nrows), m_ncols (a.m_ncols), m_count (1) + { + octave_idx_type nz = a.nnz (); + std::copy_n (a.m_data, nz, m_data); + std::copy_n (a.m_ridx, nz, m_ridx); + std::copy_n (a.m_cidx, m_ncols + 1, m_cidx); + } - octave_idx_type nnz (void) const { return m_c[m_ncols]; } + ~SparseRep (void) { delete [] m_data; delete [] m_ridx; delete [] m_cidx; } - OCTAVE_API T& elem (octave_idx_type _r, octave_idx_type _c); - - OCTAVE_API T celem (octave_idx_type _r, octave_idx_type _c) const; + octave_idx_type nzmax (void) const { return m_nzmax; } + octave_idx_type nnz (void) const { return m_cidx[m_ncols]; } - T& data (octave_idx_type i) { return m_d[i]; } + octave_idx_type rows (void) const { return m_nrows; } + octave_idx_type cols (void) const { return m_ncols; } + octave_idx_type columns (void) const { return m_ncols; } - T cdata (octave_idx_type i) const { return m_d[i]; } + OCTAVE_API T& elem (octave_idx_type r, octave_idx_type c); - octave_idx_type& ridx (octave_idx_type i) { return m_r[i]; } + OCTAVE_API T celem (octave_idx_type r, octave_idx_type c) const; + + T& data (octave_idx_type i) { return m_data[i]; } - octave_idx_type cridx (octave_idx_type i) const { return m_r[i]; } + T cdata (octave_idx_type i) const { return m_data[i]; } + + octave_idx_type& ridx (octave_idx_type i) { return m_ridx[i]; } - octave_idx_type& cidx (octave_idx_type i) { return m_c[i]; } + octave_idx_type cridx (octave_idx_type i) const { return m_ridx[i]; } - octave_idx_type ccidx (octave_idx_type i) const { return m_c[i]; } + octave_idx_type& cidx (octave_idx_type i) { return m_cidx[i]; } + + octave_idx_type ccidx (octave_idx_type i) const { return m_cidx[i]; } OCTAVE_API void maybe_compress (bool remove_zeros); @@ -129,11 +157,14 @@ OCTAVE_API bool any_element_is_nan (void) const; - private: + // Prefer nzmax. + octave_idx_type length (void) const { return m_nzmax; } + + template friend class Sparse; // No assignment! - OCTAVE_API SparseRep& operator = (const SparseRep& a); + OCTAVE_API SparseRep& operator = (const SparseRep&) = delete; }; //-------------------------------------------------------------------- @@ -151,10 +182,7 @@ } } -public: - - // !!! WARNING !!! -- these should be protected, not public. You - // should not access these data members directly! +protected: typename Sparse::SparseRep *m_rep; @@ -197,15 +225,10 @@ // Type conversion case. Preserves nzmax. template Sparse (const Sparse& a) - : m_rep (new typename Sparse::SparseRep (a.m_rep->m_nrows, a.m_rep->m_ncols, - a.m_rep->m_nzmx)), - m_dimensions (a.m_dimensions) - { - octave_idx_type nz = a.nnz (); - std::copy_n (a.m_rep->m_d, nz, m_rep->m_d); - std::copy_n (a.m_rep->m_r, nz, m_rep->m_r); - std::copy_n (a.m_rep->m_c, m_rep->m_ncols + 1, m_rep->m_c); - } + : m_rep (new typename Sparse::SparseRep (a.rows (), a.cols (), + a.nzmax (), a.data (), + a.ridx (), a.cidx ())), + m_dimensions (a.dims ()) { } // No type conversion case. Sparse (const Sparse& a) @@ -234,7 +257,7 @@ //! Amount of storage for nonzero elements. //! This may differ from the actual number of elements, see nnz(). - octave_idx_type nzmax (void) const { return m_rep->length (); } + octave_idx_type nzmax (void) const { return m_rep->nzmax (); } //! Actual number of nonzero terms. octave_idx_type nnz (void) const { return m_rep->nnz (); } @@ -472,40 +495,40 @@ OCTAVE_API Sparse transpose (void) const; - T * data (void) { make_unique (); return m_rep->m_d; } + T * data (void) { make_unique (); return m_rep->m_data; } T& data (octave_idx_type i) { make_unique (); return m_rep->data (i); } - T * xdata (void) { return m_rep->m_d; } + T * xdata (void) { return m_rep->m_data; } T& xdata (octave_idx_type i) { return m_rep->data (i); } T data (octave_idx_type i) const { return m_rep->data (i); } // FIXME: shouldn't this be returning const T*? - T * data (void) const { return m_rep->m_d; } + T * data (void) const { return m_rep->m_data; } - octave_idx_type * ridx (void) { make_unique (); return m_rep->m_r; } + octave_idx_type * ridx (void) { make_unique (); return m_rep->m_ridx; } octave_idx_type& ridx (octave_idx_type i) { make_unique (); return m_rep->ridx (i); } - octave_idx_type * xridx (void) { return m_rep->m_r; } + octave_idx_type * xridx (void) { return m_rep->m_ridx; } octave_idx_type& xridx (octave_idx_type i) { return m_rep->ridx (i); } octave_idx_type ridx (octave_idx_type i) const { return m_rep->cridx (i); } // FIXME: shouldn't this be returning const octave_idx_type*? - octave_idx_type * ridx (void) const { return m_rep->m_r; } + octave_idx_type * ridx (void) const { return m_rep->m_ridx; } - octave_idx_type * cidx (void) { make_unique (); return m_rep->m_c; } + octave_idx_type * cidx (void) { make_unique (); return m_rep->m_cidx; } octave_idx_type& cidx (octave_idx_type i) { make_unique (); return m_rep->cidx (i); } - octave_idx_type * xcidx (void) { return m_rep->m_c; } + octave_idx_type * xcidx (void) { return m_rep->m_cidx; } octave_idx_type& xcidx (octave_idx_type i) { return m_rep->cidx (i); } octave_idx_type cidx (octave_idx_type i) const { return m_rep->ccidx (i); } // FIXME: shouldn't this be returning const octave_idx_type*? - octave_idx_type * cidx (void) const { return m_rep->m_c; } + octave_idx_type * cidx (void) const { return m_rep->m_cidx; } octave_idx_type ndims (void) const { return m_dimensions.ndims (); }