addressed review comments

Author: t4c1

stan/math/prim/fun/simplex_constrain.hpp

@@ -20,7 +20,7 @@ namespace math {
  *
  * The transform is based on a centered stick-breaking process.
  *
- * @tparam T type of the vector
+ * @tparam ColVec type of the vector
  * @param y Free vector input of dimensionality K - 1.
  * @return Simplex of dimensionality K.
  */

stan/math/prim/fun/simplex_free.hpp

@@ -19,7 +19,7 @@ namespace math {
  * <p>The simplex transform is defined through a centered
  * stick-breaking process.
  *
- * @tparam T type of elements in the simplex
+ * @tparam ColVec type of the simplex (must be a column vector)
  * @param x Simplex of dimensionality K.
  * @return Free vector of dimensionality (K-1) that transforms to
  * the simplex.

stan/math/prim/fun/singular_values.hpp

@@ -13,7 +13,7 @@ namespace math {
  * <p>See the documentation for <code>svd()</code> for
  * information on the singular values.
  *
- * @tparam T type of elements in the matrix
+ * @tparam EigMat type of the matrix
  * @param m Specified matrix.
  * @return Singular values of the matrix.
  */

stan/math/prim/fun/size_mvt.hpp

@@ -20,14 +20,14 @@ namespace math {
  *   only match scalars.
  * @throw std::invalid_argument since the type is a scalar.
  */
-template <typename T, require_stan_scalar_t<T>* = nullptr>
-size_t size_mvt(const T& /* unused */) {
+template <typename ScalarT, require_stan_scalar_t<ScalarT>* = nullptr>
+size_t size_mvt(const ScalarT& /* unused */) {
   throw std::invalid_argument("size_mvt passed to an unrecognized type.");
   return 1U;
 }
 
-template <typename T, require_eigen_t<T>* = nullptr>
-size_t size_mvt(const T& /* unused */) {
+template <typename EigenT, require_eigen_t<EigenT>* = nullptr>
+size_t size_mvt(const EigenT& /* unused */) {
   return 1U;
 }
 

stan/math/prim/fun/softmax.hpp

@@ -37,7 +37,7 @@ namespace math {
  * \end{array}
  * \f$
  *
- * @tparam T type of elements in the vector
+ * @tparam ColVec type of elements in the vector
  * @param[in] v Vector to transform.
  * @return Unit simplex result of the softmax transform of the vector.
  */

stan/math/prim/fun/to_array_1d.hpp

@@ -14,14 +14,11 @@ namespace math {
 template <typename EigMat, require_eigen_t<EigMat>* = nullptr>
 inline std::vector<value_type_t<EigMat>> to_array_1d(const EigMat& matrix) {
   using T_val = value_type_t<EigMat>;
-  const Eigen::Ref<const Eigen::Matrix<T_val, EigMat::RowsAtCompileTime,
-                                       EigMat::ColsAtCompileTime>>& mat_ref
+  std::vector<T_val> result(matrix.size());
+  Eigen::Map<Eigen::Matrix<T_val, EigMat::RowsAtCompileTime,
+                           EigMat::ColsAtCompileTime>>(result.data(), matrix.rows(),
+                                                       matrix.cols())
       = matrix;
-  int matrix_size = matrix.size();
-  std::vector<T_val> result(matrix_size);
-  for (int i = 0; i < matrix_size; i++) {
-    result[i] = mat_ref.data()[i];
-  }
   return result;
 }
 

stan/math/rev/fun/sd.hpp

@@ -35,12 +35,10 @@ var sd(const T& m) {
     using T_map = std::decay_t<decltype(dtrs_map)>;
     using T_vi = promote_scalar_t<vari*, T_map>;
     using T_d = promote_scalar_t<double, T_map>;
-    vari** varis
-        = reinterpret_cast<vari**>(ChainableStack::instance_->memalloc_.alloc(
-            dtrs_map.size() * sizeof(vari*)));
-    double* partials
-        = reinterpret_cast<double*>(ChainableStack::instance_->memalloc_.alloc(
-            dtrs_map.size() * sizeof(double)));
+    vari** varis = ChainableStack::instance_->memalloc_.alloc_array<vari*>(
+        dtrs_map.size());
+    double* partials = ChainableStack::instance_->memalloc_.alloc_array<double>(
+        dtrs_map.size());
     Eigen::Map<T_vi> varis_map(varis, dtrs_map.rows(), dtrs_map.cols());
     Eigen::Map<T_d> partials_map(partials, dtrs_map.rows(), dtrs_map.cols());
 

test/unit/math/prim/fun/to_array1d_test.cpp

@@ -0,0 +1,23 @@
+#include <stan/math/prim.hpp>
+#include <test/unit/math/prim/fun/expect_matrix_eq.hpp>
+#include <gtest/gtest.h>
+#include <vector>
+#include <stdexcept>
+
+using stan::math::to_array_1d;
+
+TEST(MathMatrix, to_array_1d_matrix){
+  Eigen::MatrixXd a(3,3);
+  a << 1,2,3,4,5,6,7,8,9;
+  std::vector<double> a_correct{1,4,7,2,5,8,3,6,9};
+  std::vector<double> a_res = to_array_1d(a);
+  expect_std_vector_eq(a_res, a_correct);
+}
+
+TEST(MathMatrix, to_array_1d_matrix_block){
+  Eigen::MatrixXd a(3,3);
+  a << 1,2,3,4,5,6,7,8,9;
+  std::vector<double> a_correct{2,5,3,6};
+  std::vector<double> a_res = to_array_1d(a.block(0,1,2,2));
+  expect_std_vector_eq(a_res, a_correct);
+}