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

Pathfinder doc / behavior issues #878

Open
avehtari opened this issue Nov 15, 2023 · 10 comments
Open

Pathfinder doc / behavior issues #878

avehtari opened this issue Nov 15, 2023 · 10 comments
Assignees

Comments

@avehtari
Copy link
Contributor

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
Copy link
Member

jgabry commented Nov 15, 2023

@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
Copy link
Member

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 stan-dev/cmdstan#1221

@jgabry
Copy link
Member

jgabry commented Nov 15, 2023

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

@avehtari
Copy link
Contributor Author

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

@jgabry
Copy link
Member

jgabry commented Nov 15, 2023

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
Copy link
Member

jgabry commented Dec 12, 2023

@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
Copy link
Contributor Author

Yes

jgabry added a commit that referenced this issue Dec 13, 2023
@jgabry
Copy link
Member

jgabry commented Dec 13, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

Has this been fixed?

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

No branches or pull requests

3 participants