Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CrsMatrix raw host array constructor documented wrong #703

Closed
brian-kelley opened this issue Apr 27, 2020 · 3 comments
Closed

CrsMatrix raw host array constructor documented wrong #703

brian-kelley opened this issue Apr 27, 2020 · 3 comments

Comments

@brian-kelley
Copy link
Contributor

Comments say it takes coordinate format (values, rows, cols) but it really takes CRS format (values, rowptrs, cols).

That constructor and its doxygen:

508   /// \brief Constructor that copies raw arrays of host data in
509   ///   coordinate format.
510   ///
511   /// On input, each entry of the sparse matrix is stored in val[k],
512   /// with row index rows[k] and column index cols[k].  We assume that
513   /// the entries are sorted in increasing order by row index.
514   ///
515   /// This constructor is mainly useful for benchmarking or for
516   /// reading the sparse matrix's data from a file.
517   ///
518   /// \param label [in] The sparse matrix's label.
519   /// \param nrows [in] The number of rows.
520   /// \param ncols [in] The number of columns.
521   /// \param annz [in] The number of entries.
522   /// \param val [in] The entries.
523   /// \param rows [in] The row indices.  rows[k] is the row index of
524   ///   val[k].
525   /// \param cols [in] The column indices.  cols[k] is the column
526   ///   index of val[k].
527   /// \param pad [in] If true, pad the sparse matrix's storage with
528   ///   zeros in order to improve cache alignment and / or
529   ///   vectorization.
530   ///
531   /// FIXME (mfh 21 Jun 2013) The \c pad argument is currently not used.
532   CrsMatrix (const std::string &label,
533              OrdinalType nrows,
534              OrdinalType ncols,
535              size_type annz,
536              ScalarType* val,
537              OrdinalType* rows,
538              OrdinalType* cols,
539              bool pad = false)
540   {
541     (void) pad;
542     ctor_impl (label, nrows, ncols, annz, val, rows, cols);

ctor_impl:

873 template< typename ScalarType , typename OrdinalType, class Device, class MemoryTraits, typename SizeType >
874 void
875 CrsMatrix<ScalarType , OrdinalType, Device, MemoryTraits, SizeType >::
876 ctor_impl (const std::string &label,
877            const OrdinalType nrows,
878            const OrdinalType ncols,
879            const size_type annz,
880            ScalarType* val,
881            OrdinalType* rows,
882            OrdinalType* cols)
883 {
884   std::string str = label;
885   values = values_type (str.append (".values"), annz);
886 
887   numCols_ = ncols;
888 
889   // FIXME (09 Aug 2013) CrsArray only takes std::vector for now.
890   // We'll need to fix that.
891   std::vector<int> row_lengths (nrows, 0);
892 
893   // FIXME (mfh 21 Jun 2013) This calls for a parallel_for kernel.
894   for (OrdinalType i = 0; i < nrows; ++i) {
895     row_lengths[i] = rows[i + 1] - rows[i]; //here, it's clear that rows really means CRS row offsets
896   }
@brian-kelley
Copy link
Contributor Author

I'll fix this later today

@brian-kelley
Copy link
Contributor Author

OK, I have a fix but I ended up doing a tiny cleanup thing (removing unused parameter "pad") that changed code, so I have to run spot check. Kokkos has an issue that's blocking spot check, so I will wait for that to be fixed.

@ndellingwood
Copy link
Contributor

Cross-reference #708

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants