stan-dev / stanc3

The Stan transpiler (from Stan to C++ and beyond).
BSD 3-Clause "New" or "Revised" License
142 stars 46 forks source link

Optimization: Lazy code motion and SoA can combine to create uncompilable code #1463

Open WardBrian opened 1 day ago

WardBrian commented 1 day ago

Initially reported https://github.com/stan-dev/cmdstan/issues/1299

This stan code:

parameters {
  vector[1] x;
}
transformed parameters {
  vector[1] theta = 1 * x;
}
model {
    target += theta[1];  # using theta here seems necessary for the bug
}

When compiled with --Oexperimental, creates this C++:

      double lcm_sym8__;
      Eigen::Matrix<local_scalar_t__,-1,1> lcm_sym7__;
      current_statement__ = 1;
      auto x =
        in__.template read<
          stan::math::var_value<Eigen::Matrix<double,-1,1>>>(1);
      stan::math::var_value<Eigen::Matrix<double,-1,1>> theta =
        stan::math::var_value<Eigen::Matrix<double,-1,1>>(Eigen::Matrix<double,-1,1>::Constant(1,
                                                            std::numeric_limits<double>::quiet_NaN(
                                                              )));
      stan::model::assign(lcm_sym7__, stan::math::multiply(1, x),
        "assigning variable lcm_sym7__");

The assign here fails because lcm_sym7__ is AoS, but x (and therefor the return of multiply) is SoA.

WardBrian commented 1 day ago

Honestly, the lazy code motion pass seems to me to be so broken that I'm not even positive this is worth fixing. The very next line is

      stan::model::assign(theta, lcm_sym7__, "assigning variable theta");

Which, if you do the obvious thing and just assign directly to theta, prevents the bug. I have no idea what LCM is thinking here, and I believe this is also the pass that will move things from transformed data into e.g. the model block, which is obviously wrong. https://github.com/stan-dev/stanc3/issues/860 seems related