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

[BUG] Model crash when compiled with "--O1" #1141

Closed joewing closed 2 years ago

joewing commented 2 years ago

Current Behavior:

Compiling certain models using stanc with "--O1" generates a model that crashes with a segmentation fault. The same model appears to work fine when compiled with "--O0" or with Stan versions prior to 2.29.0. It looks like it might be related to the update of a multi-dimensional array in a loop.

Expected Behavior:

The model should run with "--O1".

Model Code

Here's what I was able to come up with for a minimal example.

transformed data {
  vector[5] inputs[2] = rep_array(zeros_vector(5), 2);
}
parameters {
  real param;
}
model {
  vector[5] local[2] = inputs;
  for(i in 1:2) {
    for(j in 1:5) {
      local[i, j] += 1;
    }
  }
  param ~ std_normal();
}

Environment:

bob-carpenter commented 2 years ago

Thanks for filing, @joewing. I couldn't figure out how to get -O1 compilation working. Could you include the exact R you used to tickle this bug?

Everything works fine with my default optimization level with cmdstan 2.29.1 under Mac OS X. But I can't figure out how to compile at different optimization level, because this command crashes:

> model <- cmdstan_model('crasher.stan', cpp_options="-O1")
Error in cpp_options$stan_threads : 
  $ operator is invalid for atomic vectors

Maybe @jgabry knows what's going on and can help with compilation. But the above looks like a bug in cmdstan_model. Here's my info

> sessionInfo()
R version 3.6.2 (2019-12-12)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS  10.16

Matrix products: default
BLAS:   /Library/Frameworks/R.framework/Versions/3.6/Resources/lib/libRblas.0.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.6/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] cmdstanr_0.4.0

loaded via a namespace (and not attached):
 [1] knitr_1.33           magrittr_2.0.1       tidyselect_1.1.0     munsell_0.5.0        colorspace_1.4-1     R6_2.5.0             rlang_0.4.11         fansi_0.4.1          dplyr_1.0.6         
[10] tools_3.6.2          grid_3.6.2           checkmate_2.0.0      data.table_1.13.6    gtable_0.3.0         xfun_0.25            utf8_1.1.4           DBI_1.1.1            posterior_0.1.4     
[19] ellipsis_0.3.2       assertthat_0.2.1     abind_1.4-5          tibble_3.1.0         lifecycle_1.0.0      crayon_1.4.1         processx_3.5.2       tensorA_0.36.2       purrr_0.3.4         
[28] farver_2.0.3         ggplot2_3.3.3        vctrs_0.3.8          ps_1.6.0             glue_1.4.2           compiler_3.6.2       pillar_1.6.0         generics_0.1.0       scales_1.1.0        
[37] backports_1.2.1      distributional_0.2.2 jsonlite_1.7.2       pkgconfig_2.0.3     
rok-cesnovar commented 2 years ago

O1 is a stanc flag, not a C++ flag, so in order to use it you need to run:

mod_opt <- cmdstan_model("model_name.stan", stanc_options = list("O1"))

Also see https://discourse.mc-stan.org/t/how-to-use-stanc3-compiler-optimization-flag-o-o1-in-cmdstanr/26365/2 for a more complete example.

bob-carpenter commented 2 years ago

Thanks, @rok-cesnovar. That worked in the sense that it crashes cmdstan as in the bug report.

O1 is a stanc flag, not a C++ flag

Does that mean it's a flag for the optimization done by stanc rather than an option passed to C++? Naming it the same thing as the C++ compiler option is confusing.

If I'm understanding correctly, the upshot is that stanc3 has a bug in its optimization. How are the optimizations being tested?

rok-cesnovar commented 2 years ago

Does that mean it's a flag for the optimization done by stanc rather than an option passed to C++?

Exactly.This is the stanc3 optimizations, see user's guide section: https://mc-stan.org/docs/2_29/stan-users-guide/optimization.html#optimization

The optimization levels (O0, O1, ... ) are new for this last release as well as the static matrix optimizations (so called varmat - struct of array vs array of struct).

Naming it the same thing as the C++ compiler option is confusing.

Indeed, there is also the Stan optimization algorithm, which adds another layer to the naming things.

If I'm understanding correctly, the upshot is that stanc3 has a bug in its optimization.

Yeah, that is my understanding as well. I am guessing there is some issue with the static matrix/varmat, given that the param is given the Struct of Array metadata (struct of array = varmat). This is the only difference in the MIR between the the O0 and O1 optimization levels.

See here and the diff check.

cc: @SteveBronder you are probably the most proficient in this stuff so tagging you, if you havent seen this. Sorry for the noise if you did.

SteveBronder commented 2 years ago

@rok-cesnovar just got to this and figured it out. It's because of the no init NA optimization causing the code here in the generated C++. I think this is not a compiler bug, but a bug with assign.

        std::vector<Eigen::Matrix<local_scalar_t__, -1, 1>> local;
        current_statement__ = 2;
        stan::model::assign(local, inputs, "assigning variable local");

The line for assign I think is using an iterative assignment for each element in local, but we need to do one big block assignment in order for the above to be valid code.

stan::model::assign(local, inputs, "assigning variable local");

This should be a simple fix I'll put up something today