stan-dev / math

The Stan Math Library is a C++ template library for automatic differentiation of any order using forward, reverse, and mixed modes. It includes a range of built-in functions for probabilistic modeling, linear algebra, and equation solving.
https://mc-stan.org
BSD 3-Clause "New" or "Revised" License
744 stars 187 forks source link

wishart_rng accepts non-positive definite scale matrices #1903

Open ecmerkle opened 4 years ago

ecmerkle commented 4 years ago

Description

In supplying a scale matrix (parameter) to wishart_rng, one can use a matrix that is not positive definite and obtain results. Is this expected behavior? It differs from a function like multi_normal_rng, which checks whether or not the supplied covariance matrix is positive definite and fails if not.

Example

The zip file contains an R file and stan file. The R file runs the stan file and illustrates the issue:

testwish.zip

Expected Output

I expected behavior similar to multi_normal_rng, which fails when a non-positive definite covariance matrix is supplied.

Current Version:

Here is my R sessionInfo():

R version 4.0.0 (2020-04-24) Platform: x86_64-pc-linux-gnu (64-bit) Running under: Ubuntu 18.04.4 LTS

Matrix products: default BLAS: /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.7.1 LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.7.1

locale: [1] LC_CTYPE=en_US.UTF-8 LC_NUMERIC=C
[3] LC_TIME=en_US.UTF-8 LC_COLLATE=en_US.UTF-8
[5] LC_MONETARY=en_US.UTF-8 LC_MESSAGES=en_US.UTF-8
[7] LC_PAPER=en_US.UTF-8 LC_NAME=C
[9] LC_ADDRESS=C LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

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

other attached packages: [1] rstan_2.19.3 ggplot2_3.3.0 StanHeaders_2.21.0-1

loaded via a namespace (and not attached): [1] Rcpp_1.0.4.6 magrittr_1.5 tidyselect_1.1.0 munsell_0.5.0
[5] colorspace_1.4-1 R6_2.4.1 rlang_0.4.6 fansi_0.4.1
[9] dplyr_0.8.5 tools_4.0.0 parallel_4.0.0 pkgbuild_1.0.8
[13] grid_4.0.0 gtable_0.3.0 loo_2.2.0 cli_2.0.2
[17] withr_2.2.0 matrixStats_0.56.0 ellipsis_0.3.1 assertthat_0.2.1
[21] tibble_3.0.1 lifecycle_0.2.0 crayon_1.3.4 processx_3.4.2
[25] gridExtra_2.3 purrr_0.3.4 callr_3.4.3 vctrs_0.3.0
[29] ps_1.3.3 inline_0.3.15 glue_1.4.1 compiler_4.0.0
[33] pillar_1.4.4 prettyunits_1.1.1 scales_1.1.1 stats4_4.0.0
[37] pkgconfig_2.0.3

bob-carpenter commented 4 years ago

Thanks much for reporting with a reproducible example. That's indeed a bug. We should be checking for positive definiteness after its factored. I just checked the code and see that we're not.