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

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

Open
andrjohns opened this issue Nov 30, 2023 · 5 comments
Open

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

andrjohns opened this issue Nov 30, 2023 · 5 comments

Comments

@andrjohns
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

Ah got it, thanks!

@andrjohns
Copy link
Contributor Author

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?

@andrjohns andrjohns reopened this Nov 30, 2023
@WardBrian
Copy link
Member

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
Copy link
Contributor Author

Ahhh that makes sense, fair enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants