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/Inconsistency] Please Convert Warning 'Messages' to Warnings #1039

Open
emstruong opened this issue Nov 19, 2024 · 5 comments
Open

[Bug/Inconsistency] Please Convert Warning 'Messages' to Warnings #1039

emstruong opened this issue Nov 19, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@emstruong
Copy link

emstruong commented Nov 19, 2024

Describe the bug
Some of the 'warnings' from cmdstanr are actually 'messages' and not warnings. This can be messy in Monte Carlo simulations.

cmdstanr/R/utils.R

Lines 287 to 310 in bceb482

message(
"Warning: ", num_divergences, " of ", num_draws,
" (", (base::format(round(percentage_divergences, 0), nsmall = 1)), "%)",
" transitions ended with a divergence.\n",
"See https://mc-stan.org/misc/warnings for details.\n"
)
}
}
invisible(unname(num_divergences_per_chain))
}
check_max_treedepth <- function(post_warmup_sampler_diagnostics, metadata) {
num_max_treedepths_per_chain <- NULL
if (!is.null(post_warmup_sampler_diagnostics)) {
treedepths <- posterior::extract_variable_matrix(post_warmup_sampler_diagnostics, "treedepth__")
num_max_treedepths_per_chain <- apply(treedepths, 2, function(x) sum(x >= metadata$max_treedepth))
num_max_treedepths <- sum(num_max_treedepths_per_chain)
num_draws <- length(treedepths)
if (!is.na(num_max_treedepths) && num_max_treedepths > 0) {
percentage_max_treedepths <- 100 * num_max_treedepths / num_draws
message(
"Warning: ", num_max_treedepths, " of ", num_draws, " (", (base::format(round(percentage_max_treedepths, 0), nsmall = 1)), "%)",
" transitions hit the maximum treedepth limit of ", metadata$max_treedepth,".\n",
"See https://mc-stan.org/misc/warnings for details.\n"

Expected behavior
I expected the warning messages to actually be warnings and I would really appreciate if they were warnings

Operating system
Ubuntu 22.04

CmdStanR version number
0.8.1

@emstruong emstruong added the bug Something isn't working label Nov 19, 2024
@jgabry
Copy link
Member

jgabry commented Nov 20, 2024

Hi @emstruong thanks for the suggestions. Can you say a bit more about what makes this "messy" in Monte Carlo simulations? In the past we've had complaints about too many warnings, which is one reason this is a message.

@emstruong
Copy link
Author

Ah, I see... Well, at least in terms of monitoring simulations from library(SimDesign), warning + error messages are logged to monitor the progress/state of the simulation. Things that are not warnings or errors clog up the logs of what's happening. Also, there's the ability to manage/suppress specific warnings + errors that are already known to occur.

Regarding the complaints, couldn't its verbosity be controlled via an argument? Or perhaps whether it's a message or a warning be controlled, as in lme4? https://github.com/lme4/lme4/blob/779bc7098b19b9aef5eab6df2ae263e06e24ddc4/R/lmerControl.R

@jgabry
Copy link
Member

jgabry commented Nov 20, 2024

Well, at least in terms of monitoring simulations from library(SimDesign), warning + error messages are logged to monitor the progress/state of the simulation. Things that are not warnings or errors clog up the logs of what's happening.

I see, I didn't know that (never used SimDesign before).

For the moment one thing you can do is set diagnostics=NULL when calling the sample() method, which will prevent any diagnostic messages from being printed and clogging up the logs. Then if you want to know if there were divergences you can retrieve the number of divergences with fit$diagnostic_summary(quiet=TRUE). I suppose you could even create your own wrapper function to use in your simulations if you want actual warnings:

# I haven't tested this but something like this should work
fit_model <- function(model, ...) {
  fit <- model$sample(..., diagnostics = NULL) # suppress diagnostic messages
  divergences <- sum(fit$diagnostic_summary(quiet=TRUE)$num_divergent) 
  if (divergences > 0) {
    warning("There were ", divergences, " divergences")
  }
  return(fit)
}

Or perhaps whether it's a message or a warning be controlled, as in lme4?

Yeah, I think something like this would be reasonable. I'd prefer to give the user an option to make it a warning rather than forcing it by changing (and hardcoding) all the messages to warnings, since at this point people are used to the current situation. But I agree an option would be good.

@emstruong
Copy link
Author

I see, I didn't know that (never used SimDesign before).

Ah... Well I think what I was meaning was that when a lot of different models are repeatedly fitted within a single session, as may be the case in multiverse analyses, large production cases, or Monte Carlo Simulation designs, I think the small things like message, warning and error handling can become quite important. As there's not much of any other way to automate interacting with the model... Which I sense is largely the opposite of the way Bayesian analysis should be--meaningful priors and thoughtful investigation of posteriors.

For the moment one thing you can do is set diagnostics=NULL when calling the sample() method, which will prevent any diagnostic messages from being printed and clogging up the logs. Then if you want to know if there were divergences you can retrieve the number of divergences with fit$diagnostic_summary(quiet=TRUE). I suppose you could even create your own wrapper function to use in your simulations if you want actual warnings:

# I haven't tested this but something like this should work
fit_model <- function(model, ...) {
  fit <- model$sample(..., diagnostics = NULL) # suppress diagnostic messages
  divergences <- sum(fit$diagnostic_summary(quiet=TRUE)$num_divergent) 
  if (divergences > 0) {
    warning("There were ", divergences, " divergences")
  }
  return(fit)
}

I think this could work--although it may or may not work with brms. I'll have to check

Yeah, I think something like this would be reasonable. I'd prefer to give the user an option to make it a warning rather than forcing it by changing (and hardcoding) all the messages to warnings, since at this point people are used to the current situation. But I agree an option would be good.

I agree.

@jgabry
Copy link
Member

jgabry commented Nov 20, 2024

Yeah I'm not sure if brms gives you access to the CmdStanR fitted model object. If not, you could use brms::make_stancode() and brms::make_standata() to get the Stan code and the data in the right format and then use CmdStanR itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants