stan-dev / cmdstanr

CmdStanR: the R interface to CmdStan
https://mc-stan.org/cmdstanr/
Other
143 stars 63 forks source link

Pathfinder doc / behavior issues #878

Open avehtari opened 11 months ago

avehtari commented 11 months ago

I titled this issue as generic Pathfinder doc / behavior issues, not to make separate for each small issue

$pathfinder() doc says

   draws: (positive integer) Number of draws to return after performing
          pareto smooted importance sampling (PSIS). This must be
          smaller than ‘single_path_draws * num_paths’.

Current behavior is that this is by default 1000 which can be bigger than ‘single_path_draws num_paths’ and also it can be set to value which is bigger than ‘single_path_draws num_paths’, without any warning.

jgabry commented 11 months ago

@avehtari Are you proposing just that we add a warning or change the behavior?

@WardBrian @SteveBronder Is this also the case with the cmdstanpy implementation or did we end up doing something different here accidentally?

WardBrian commented 11 months ago

We had discussions about automatically adjusting the num_single_draws to prevent this but Steve argued successfully that this would be too magic. Cmdstan got a warning for it in https://github.com/stan-dev/cmdstan/pull/1221

jgabry commented 11 months ago

Oh ok great, so this warning will get handled in CmdStan itself.

avehtari commented 11 months ago

I'm fine that the document is updated to reflect that it doesn't need to be smaller

jgabry commented 11 months ago

Ok instead of "This must be smaller than ‘single_path_draws num_paths’." should we just say "This should be smaller than ‘single_path_draws num_paths’ or CmdStan will throw a warning." Or something else? (Currently it won't throw a warning but sounds like next version will)

jgabry commented 10 months ago

@avehtari are you ok with changing the doc to "This should be smaller than ‘single_path_draws * num_paths’ or CmdStan will throw a warning."? (although the warning won't come until the next version of CmdStan)

avehtari commented 10 months ago

Yes

jgabry commented 10 months ago

Ok I updated the doc to

This should be smaller than single_path_draws * num_paths (future versions of CmdStan will throw a warning).

I will leave this issue open and wait to close it until the CmdStan with the warning is released (at that time I'll update the doc again).

avehtari commented 10 months ago

Adding here another pathfinder issue save_single_paths option does not seem to be working (installed the latest cmdstanr from github). With save_single_paths=TRUE (or =1) there are no files for individual paths and the only csv shows

#     save_single_paths = 0 (Default)
avehtari commented 5 months ago

Has this been fixed?