stan-dev / stanc3

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

[BUG] SoA request is ignored with external c++ #1379

Open andrjohns opened 7 months ago

andrjohns commented 7 months ago

It looks like stanc ignores the -fsoa flag if any undefined functions are used in the likelihood. Given the following model:

functions {
  real undefined_fun(vector x);
}
data {
  int Npars;
}

parameters {
  vector[Npars] pars;
}

model {
  target += undefined_fun(pars);
}

I get the following generated c++:

...
Eigen::Matrix<local_scalar_t__,-1,1> pars =
  Eigen::Matrix<local_scalar_t__,-1,1>::Constant(Npars, DUMMY_VAR__);
...

If I remove the undefined_fun() call from the likelihood:

functions {
  real undefined_fun(vector x);
}
data {
  int Npars;
}

parameters {
  vector[Npars] pars;
}

model {
  target += sum(pars);
}

I instead get:

...
stan::math::var_value<Eigen::Matrix<double,-1,1>> pars =
  stan::math::var_value<Eigen::Matrix<double,-1,1>>(Eigen::Matrix<double,-1,1>::Constant(Npars,
                                                      std::numeric_limits<double>::quiet_NaN(
                                                        )));
...

Both models were called with ./mac-stanc ./soa_test.stan -fsoa --allow-undefined using the Oct 23 nightly (the latest release)

WardBrian commented 7 months ago

I think this is intentional behavior. We currently only support SoA in user defined functions if we can inline them (see #1237), and it is unclear whether or not the user providing external C++ would desire SoA or AoS inputs.

The later is perhaps something we could fix with annotations once we implement them

andrjohns commented 7 months ago

Ah got it, thanks!

andrjohns commented 7 months ago

it is unclear whether or not the user providing external C++ would desire SoA or AoS inputs.

Actually, I might reopen sorry. I think that concern is only relevant if the user has passed the --O1 flag. If they're using -fsoa then it should be assumed that they specifically want SoA.

Would a compromise be to allow this with the -fsoa flag but output a warning/message?

WardBrian commented 7 months ago

At the moment, the optimizer has no way of knowing whether the reason it is doing the memory pattern optimizations is because it is from -fsoa or --O1 -- both those flags just set the same boolean internally. I'd be a bit hesitant to make them have different behavior

andrjohns commented 7 months ago

Ahhh that makes sense, fair enough