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

[Waiting on Stan PR] Adds psis_resample and calculate_lp as options for pathfinder #1234

Merged
merged 5 commits into from
Jan 8, 2024

Conversation

SteveBronder
Copy link
Contributor

Submisison Checklist

  • Run tests: ./runCmdStanTests.py src/test
  • Declare copyright holder and open-source license: see below

Summary:

Adds psis_resample and calculate_lp as options for pathfinder

Intended Effect:

How to Verify:

Do we need tests for these? The options are tested in Stan so I think it would be redundant here

Side Effects:

Documentation:

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company):

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

" from individual pathfinders. If false, returns num_paths * num_draws"
" samples",
true));
_subarguments.push_back(new arg_single_bool(
"calculate_lp", "If true, individual pathfinders do not calculate lp."
"calculate_lp",
"If true, individual pathfinders do not calculate lp."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems counter-intuitive - true == do it, false == don't do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I mixed up the docs, lp values are calculated for true and not for false

@WardBrian WardBrian mentioned this pull request Jan 8, 2024
26 tasks
@SteveBronder SteveBronder reopened this Jan 8, 2024
@SteveBronder SteveBronder marked this pull request as ready for review January 8, 2024 14:49
Copy link
Member

@WardBrian WardBrian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Just noting for our own purposes that these need doc updates during the RC period

@WardBrian WardBrian merged commit 92186e3 into develop Jan 8, 2024
1 check passed
@WardBrian WardBrian deleted the feature/pathfinder-resampling-lpcalc branch January 8, 2024 15:38
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

Successfully merging this pull request may close these issues.

4 participants