Mercurial > octave

changeset 29927:6a8642d310c8

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
style fixes for Sparse<T> 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<T>): Make data members protected instead of public. (SparseRep): Declare Sparse class as friend. (SparseRep): New constructors that accept data elements directly. (Sparse<T>::Sparse (const Sparse<U>&)): Simplify using new SparseRep constructor.
author John W. Eaton <jwe@octave.org>
date Thu, 29 Jul 2021 20:43:13 -0400
parents 6a39ac893c9d
children 2420f8f62ebe
files liboctave/array/Sparse.cc liboctave/array/Sparse.h
diffstat 2 files changed, 144 insertions(+), 121 deletions(-) [+]
line wrap: on
line diff
--- 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 <typename T>
 OCTAVE_API
 T&
-Sparse<T>::SparseRep::elem (octave_idx_type _r, octave_idx_type _c)
+Sparse<T>::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 <typename T>
 OCTAVE_API
 T
-Sparse<T>::SparseRep::celem (octave_idx_type _r, octave_idx_type _c) const
+Sparse<T>::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 <typename T>
@@ -145,32 +145,32 @@
 void
 Sparse<T>::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<T>::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 <typename T>
@@ -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<T>::SparseRep (new_nr, new_nc, new_nzmx);
+  m_rep = new typename Sparse<T>::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 <typename T>
@@ -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<T>::SparseRep (nr, nc, new_nzmx);
+      new_nzmax++;
+
+  m_rep = new typename Sparse<T>::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<T>::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<void *> (m_rep->m_d) << "\n"
-     << prefix << "m_rep->ridx:    " << static_cast<void *> (m_rep->m_r) << "\n"
-     << prefix << "m_rep->cidx:    " << static_cast<void *> (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";
 }
 
--- 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<octave_idx_type> 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 <typename U>
+    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 <typename U> 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<T>::SparseRep *m_rep;
 
@@ -197,15 +225,10 @@
   // Type conversion case.  Preserves nzmax.
   template <typename U>
   Sparse (const Sparse<U>& a)
-    : m_rep (new typename Sparse<T>::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<T>::SparseRep (a.rows (), a.cols (),
+                                                a.nzmax (), a.data (),
+                                                a.ridx (), a.cidx ())),
+      m_dimensions (a.dims ()) { }
 
   // No type conversion case.
   Sparse (const Sparse<T>& 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<T> 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 (); }