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

moment_match argument in add_criterion ignored #1323

Closed
andymilne opened this issue Mar 19, 2022 · 9 comments
Closed

moment_match argument in add_criterion ignored #1323

andymilne opened this issue Mar 19, 2022 · 9 comments
Labels
Milestone

Comments

@andymilne
Copy link

Not sure if the error is due to brms or loo. I have brms 2.16.3 and loo 2.5.0.

If I run the following

mdl3redo3 <- 
  add_criterion(
    mdl3redo3, 
    "loo",
    nsamples = 3000,
    moment_match = TRUE)
loo(mdl3redo3)

I get

Warning message:
Found 12 observations with a pareto_k > 0.7 in model 'mdl3redo3'. It is recommended to set 'moment_match = TRUE' in order to perform moment matching for problematic observations.  

which suggests that the moment_match = TRUE argument in add_criterion has been ignored.

@paul-buerkner
Copy link
Owner

Can you try it with the brms dev version from github? I think I already fixed it there.

@andymilne
Copy link
Author

andymilne commented Mar 20, 2022

Something weird is going on with loo. Using the dev version of brms

prior1 <- prior(normal(0,10), class = b) +
  prior(cauchy(0,2), class = sd)
fit1 <- brm(count ~ zAge + zBase * Trt + (1|patient),
            data = epilepsy, family = poisson(), prior = prior1)
fit1 <- add_criterion(fit1, "loo", moment_match = TRUE)

crashes R. On the other hand,

fit1 <- add_criterion(fit1, "loo")

does not crash R and gives the reasonable warning

Warning message:
Found 8 observations with a pareto_k > 0.7 in model 'fit1'. It is recommended to set 'moment_match = TRUE' in order to perform moment matching for problematic observations.

However, following that with

fit1 <- add_criterion(fit1, "loo", moment_match = TRUE, overwrite = TRUE)

seems to do nothing (it completes instantaneously) and loo(fit1) shows moment matching has not been performed.

I am on macOS 12.2.1, in case relevant.

@paul-buerkner
Copy link
Owner

Once added, add_criterion will not update the loo object unlessed forced to via overwrite = TRUE. The crash problem is likely one of rstan and you may want to try installing rstan 2.19.3 (from CRAN archive) or the dev version via

remove.packages(c("StanHeaders", "rstan"))
install.packages("StanHeaders", repos = c("https://mc-stan.org/r-packages/", getOption("repos")),
                 type = "source")
install.packages("rstan", repos = c("https://mc-stan.org/r-packages/", getOption("repos")),
                 type = "source")

@andymilne
Copy link
Author

I already have rstan 2.26.8 installed. Note that I did use overwrite = TRUE.

@paul-buerkner
Copy link
Owner

Ok, let me check.

@paul-buerkner paul-buerkner reopened this Mar 20, 2022
@paul-buerkner paul-buerkner added this to the brms 2.16.0++ milestone Mar 21, 2022
paul-buerkner added a commit that referenced this issue Apr 7, 2022
@paul-buerkner
Copy link
Owner

paul-buerkner commented Apr 7, 2022

Should now be fixed. There were two things going on.

  1. You need to specify in brm(..., save_pars = save_pars(all = TRUE)) to ensure that moment matching works in the first place. It should have given you an informative error message but perhaps that was supressed for some reason (upong crahsing).

  2. There was a bug that caused overwrite not to work as expected in some cases. This has now been fixed.

Thank you for opening this issue! Closing it now.

@andymilne
Copy link
Author

I think this is still not working. If I run

prior1 <- prior(normal(0,10), class = b) +
    prior(cauchy(0,2), class = sd)
fit1 <- brm(count ~ zAge + zBase * Trt + (1|patient),
            data = epilepsy, family = poisson(), prior = prior1,
            save_pars = save_pars(all = TRUE))
fit1 <- add_criterion(fit1, "loo", moment_match = TRUE)

I get the warning

Warning messages:
1: Some Pareto k diagnostic values are too high. See help('pareto-k-diagnostic') for details.
 
2: Found 1 observations with a pareto_k > 0.7 in model 'fit1'. It is recommended to set 'reloo = TRUE' in order to calculate the ELPD without the assumption that these observations are negligible. This will refit the model 1 times to compute the ELPDs for the problematic observations directly. 

which suggests that moment_match hasn't been performed for some reason.

@paul-buerkner
Copy link
Owner

It did work if you compare before and after moment matching (4 vs 1 problematic observation). Moment matching is not perfect so it may not always succeed in fixing problems with all observations.

@andymilne
Copy link
Author

The number of problematic observations changes every time the model is run so is pretty random. Is there any way I can check the fitted model to get confirmation moment matching has been performed? I am a bit sceptical, so it would be good to get reassurance.

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

No branches or pull requests

2 participants