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

num_chains argument fails silently for unsupported algorithms, still creates output files #1057

Closed
WardBrian opened this issue Nov 12, 2021 · 11 comments · Fixed by #1102
Closed
Labels

Comments

@WardBrian
Copy link
Member

WardBrian commented Nov 12, 2021

Summary:

When algorithm=fixed_param (either implicitly as is being removed in #1054 or explicitly) and num_chains is set to a value other than 1, multiple files are created but only the first contains any draws.

Reproducible Steps:

Model:

data { 
  int<lower=0> N;
  real<lower=0,upper=1> theta;
} 
generated quantities {
  int theta_rep = 0;
  for (n in 1:N)
    theta_rep += bernoulli_rng(theta);
}

data:

{
"N":300,
"theta": 0.3
}

command:
./bernoulli_datagen sample num_chains=7 algorithm=fixed_param data file=data.json

Current Output:

Files output_#.csv for 1 to 7. File 1 contains draws, remainder contain only the CmdStan header comment with no csv data.

Expected Output:

Either this should produce an error or each file should contain draws

Additional Information:

Related:
stan-dev/cmdstanpy#495

Current Version:

v2.28.1

@rok-cesnovar
Copy link
Member

num_chains is currently only supported with NUTS + dense_e or diag_e. Anything else is not supported. A helpful error message would be nice though.

@WardBrian
Copy link
Member Author

Silently ignoring that it was requested is pretty bad, yeah. The fact that it outputs 7 files still is even worse, especially when combined with the current behavior that fixed_params can be turned on without the user asking for it

@nsiccha
Copy link

nsiccha commented Nov 22, 2021

Same problem for nuts without adaptation.

@rok-cesnovar
Copy link
Member

rok-cesnovar commented Nov 22, 2021

@nsiccha so your model has parameters and you set sample adapt engaged=0? I tried that with the bernoulli example and it seems to work fine. Can you provide a bit more detail?

@nsiccha
Copy link

nsiccha commented Nov 22, 2021

@rok-cesnovar Yes, num_chains > 1 and sample adapt engaged=0 with the bernoulli example behaves exactly the same way as Brian describes above:

Multiple files are created, only the first contains draws and no error is reported.

I doubt that it can work for you.

First, there is a hidden error message describing this lacking feature but which doesn't get printed here:

throw std::invalid_argument(
"num_chains can currently only be used for NUTS with adaptation "
"and dense_e or diag_e metric");

Second, the calls to non-adapting nuts extract only the first "context", see e.g. here:
return_code = stan::services::sample::hmc_nuts_diag_e(
model, *(init_contexts[0]), random_seed, id, init_radius,
num_warmup, num_samples, num_thin, save_warmup, refresh, stepsize,
stepsize_jitter, max_depth, interrupt, logger, init_writers[0],
sample_writers[0], diagnostic_writers[0]);

Finally, there's simply no mention at all of num_chains outside of the adapting nuts calls:
https://github.com/stan-dev/stan/search?q=num_chains+path%3Asrc%2Fstan%2Fservices

@rok-cesnovar
Copy link
Member

Ah yes, this is simply because num_chains > 1 is not supported when using NUTS without adaptation. Not sure why the throw is not registered though. This is annoying for sure.

@nsiccha
Copy link

nsiccha commented Nov 22, 2021

Well I'll simply not use it till everything is ready, so no big deal for me. 🤷

@rok-cesnovar
Copy link
Member

That sounds reasonable. The error messaging will definitely be improved for the next release, not sure on providing multiple chains within a single executable for all services. That will probably be extended more slowly.

@WardBrian WardBrian changed the title Bad interaction between fixed_params sampler and num_chains argument num_chains argument fails silently for unsupported algorithms, still creates output files Nov 22, 2021
@WardBrian
Copy link
Member Author

Updated the title to reflect that this isn't just a fixed_param issue but any unsupported sampler

@WardBrian
Copy link
Member Author

@rok-cesnovar will this be improved before 2.29?

@rok-cesnovar
Copy link
Member

Yeah, should have time to expand error messaging this week.

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

Successfully merging a pull request may close this issue.

3 participants