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

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

Open
WardBrian opened this issue Oct 30, 2024 · 1 comment
Labels
bug Something isn't working cpp-codegen optimization

Comments

@WardBrian
Copy link
Member

Initially reported stan-dev/cmdstan#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 WardBrian added bug Something isn't working cpp-codegen optimization labels Oct 30, 2024
@WardBrian
Copy link
Member Author

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. #860 seems related

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cpp-codegen optimization
Projects
None yet
Development

No branches or pull requests

1 participant