Fix multiple unconstrained params being read incorrectly in log_prob

src/cmdstan/command_helper.hpp

@@ -528,7 +528,7 @@ Eigen::VectorXd get_laplace_mode(const std::string &fname,
  * @param model Stan model
  * @return vector of vectors of parameter estimates
  */
-std::vector<std::vector<double>> get_uparams_r(
+inline std::vector<std::vector<double>> get_uparams_r(
     const std::string &fname, const stan::model::model_base &model) {
   size_t u_params_cols = 0;
   size_t u_params_rows = 0;
@@ -559,8 +559,8 @@ std::vector<std::vector<double>> get_uparams_r(
   std::vector<std::vector<double>> params_r_ind(
       u_params_rows, std::vector<double>(u_params_cols));
   size_t idx = 0;
-  for (size_t i = 0; i < u_params_rows; ++i) {
-    for (size_t j = 0; j < u_params_cols; ++j) {
+  for (size_t j = 0; j < u_params_cols; ++j) {
+    for (size_t i = 0; i < u_params_rows; ++i) {
       params_r_ind[i][j] = *(u_params_r.data() + idx);
       ++idx;
     }
@@ -577,9 +577,10 @@ std::vector<std::vector<double>> get_uparams_r(
  * @param params_set array of unconstrained parameter values
  * @
  */
-void services_log_prob_grad(const stan::model::model_base &model, bool jacobian,
-                            std::vector<std::vector<double>> &params_set,
-                            stan::callbacks::writer &output) {
+inline void services_log_prob_grad(const stan::model::model_base &model,
+                                   bool jacobian,
+                                   std::vector<std::vector<double>> &params_set,
+                                   stan::callbacks::writer &output) {
   // header
   std::vector<std::string> p_names{"lp__"};
   model.unconstrained_param_names(p_names, false, false);

src/test/interface/log_prob_test.cpp

@@ -1,11 +1,8 @@
 #include <test/utility.hpp>
 #include <gtest/gtest.h>
-#include <fstream>
 #include <string>
-#include <stdexcept>
 
 using cmdstan::test::convert_model_path;
-using cmdstan::test::multiple_command_separator;
 using cmdstan::test::parse_sample;
 using cmdstan::test::run_command;
 using cmdstan::test::run_command_output;
@@ -30,6 +27,10 @@ class CmdStan : public testing::Test {
                                        "bern_unconstrained_params_short.json"};
     bern_constrained_params_short
         = {"src", "test", "test-models", "bern_constrained_params_short.json"};
+    bern_unconstrained_params_multi_json
+        = {"src", "test", "test-models", "bern_unconstrained_multi.json"};
+    bern_unconstrained_params_multi_rdump
+        = {"src", "test", "test-models", "bern_unconstrained_multi.R"};
     test_output = {"test", "output.csv"};
     simplex_model = {"src", "test", "test-models", "simplex_model"};
     simplex_constrained_bad_csv
@@ -47,6 +48,9 @@ class CmdStan : public testing::Test {
   std::vector<std::string> bern_constrained_params_csv;
   std::vector<std::string> bern_unconstrained_params_short;
   std::vector<std::string> bern_constrained_params_short;
+  std::vector<std::string> bern_unconstrained_params_multi_json;
+  std::vector<std::string> bern_unconstrained_params_multi_rdump;
+
   std::vector<std::string> test_output;
   std::vector<std::string> simplex_model;
   std::vector<std::string> simplex_constrained_csv;
@@ -73,6 +77,32 @@ TEST_F(CmdStan, log_prob_uparams_rdump) {
   ASSERT_EQ(values.size() % names.size(), 0);
   ASSERT_EQ(names[0].compare(0, 2, std::string("lp")), 0);
   ASSERT_EQ(names[1].compare(0, 2, std::string("g_")), 0);
+
+  ASSERT_FLOAT_EQ(values[0], -26.1950918);
+}
+
+TEST_F(CmdStan, log_prob_uparams_multi_rdump) {
+  std::stringstream ss;
+  ss << convert_model_path(bern_log_prob_model)
+     << " data file=" << convert_model_path(bern_data)
+     << " output file=" << convert_model_path(test_output)
+     << " method=log_prob unconstrained_params="
+     << convert_model_path(bern_unconstrained_params_multi_rdump);
+  std::string cmd = ss.str();
+  run_command_output out = run_command(cmd);
+  ASSERT_FALSE(out.hasError);
+  std::vector<std::string> config;
+  std::vector<std::string> header;
+  std::vector<double> values;
+  parse_sample(convert_model_path(test_output), config, header, values);
+  std::vector<std::string> names;
+  boost::split(names, header[0], boost::is_any_of(","),
+               boost::token_compress_on);
+  ASSERT_EQ(values.size() % names.size(), 0);
+
+  // log_p values calculated externally
+  ASSERT_FLOAT_EQ(values[0], -14.90125);
+  ASSERT_FLOAT_EQ(values[names.size()], -17.19926);
 }
 
 TEST_F(CmdStan, log_prob_cparams_rdump) {
@@ -93,6 +123,8 @@ TEST_F(CmdStan, log_prob_cparams_rdump) {
   boost::split(names, header[0], boost::is_any_of(","),
                boost::token_compress_on);
   ASSERT_TRUE(values.size() % names.size() == 0);
+
+  ASSERT_FLOAT_EQ(values[0], -26.1950918);
 }
 
 TEST_F(CmdStan, log_prob_uparams_json) {
@@ -113,6 +145,32 @@ TEST_F(CmdStan, log_prob_uparams_json) {
   boost::split(names, header[0], boost::is_any_of(","),
                boost::token_compress_on);
   ASSERT_EQ(values.size() % names.size(), 0);
+
+  ASSERT_FLOAT_EQ(values[0], -26.1950918);
+}
+
+TEST_F(CmdStan, log_prob_uparams_multi_json) {
+  std::stringstream ss;
+  ss << convert_model_path(bern_log_prob_model)
+     << " data file=" << convert_model_path(bern_data)
+     << " output file=" << convert_model_path(test_output)
+     << " method=log_prob unconstrained_params="
+     << convert_model_path(bern_unconstrained_params_multi_json);
+  std::string cmd = ss.str();
+  run_command_output out = run_command(cmd);
+  ASSERT_FALSE(out.hasError);
+  std::vector<std::string> config;
+  std::vector<std::string> header;
+  std::vector<double> values;
+  parse_sample(convert_model_path(test_output), config, header, values);
+  std::vector<std::string> names;
+  boost::split(names, header[0], boost::is_any_of(","),
+               boost::token_compress_on);
+  ASSERT_EQ(values.size() % names.size(), 0);
+
+  // log_p values calculated externally
+  ASSERT_FLOAT_EQ(values[0], -24.528835);
+  ASSERT_FLOAT_EQ(values[names.size()], -35.035283);
 }
 
 TEST_F(CmdStan, log_prob_cparams_json) {
@@ -133,6 +191,7 @@ TEST_F(CmdStan, log_prob_cparams_json) {
   boost::split(names, header[0], boost::is_any_of(","),
                boost::token_compress_on);
   ASSERT_EQ(values.size() % names.size(), 0);
+  ASSERT_FLOAT_EQ(values[0], -26.1950918);
 }
 
 TEST_F(CmdStan, log_prob_cparams_csv) {

src/test/test-models/bern_unconstrained_multi.R

@@ -0,0 +1,14 @@
+params_r <- structure(c(-1.7826899385311055, 0.058362903176109024, -1.8870574951389159,
+0.32182653494850982, -0.33279912821578023, 1.1755011233138752,
+0.548323677676959, -0.3390893619749813, -0.36966005408259461,
+-1.0469050778091906, -2.3571410564924653, -0.36226465437324951,
+0.18476348400049031, 0.3538173513390801, -0.29572060754310731,
+0.75118275172385274, -0.13441108926989107, -0.50208301598326377,
+0.8422549630783327, -0.85559862482538085, 0.22163915310082957,
+-0.0967955260147986, -0.42955366613672469, -1.2114667038261244,
+-0.22780372272202434, -1.5982657722626148, 0.30269019545481407,
+0.72620422198392964, 0.179903457308519, -2.599292902623854, -1.60779131334911,
+0.046478898063598527, -0.60666582204989516, 0.44509877260725766,
+0.2512029348511659, -0.99552555982651314, 1.1981866907239958,
+-0.62839111843893136), .Dim = c(2, 19))
+

src/test/test-models/bern_unconstrained_multi.json

@@ -0,0 +1,16 @@
+{
+  "params_r": [
+    [
+      1.35584153, 0.27525052, 0.96573, -2.16005226, -1.37546212, -0.11006157,
+      0.68632036, 0.89054693, 1.01844639, -1.63940773, -1.42824144, -0.58972464,
+      -0.27810713, 0.49700114, -0.66268587, -0.05818367, 0.21485661, -0.8569392,
+      1.62690887
+    ],
+    [
+      2.80681384, -0.50342205, -0.064571, -1.02065775, 0.96462088, -1.78105829,
+      0.70339144, -0.13549738, 0.96043048, 1.43833813, 0.39750983, -0.59743302,
+      -1.34201349, 1.73491275, -0.70208818, -0.30521858, -1.58527664,
+      0.02588404, 0.89910165
+    ]
+  ]
+}