Skip to content

Conversation

penelopeysm
Copy link
Member

@penelopeysm penelopeysm commented Sep 30, 2025

This should, hopefully, document a bunch of things that were not documented.

Closes #647.

Copy link
Contributor

Preview the changes: https://turinglang.org/docs/pr-previews/654
Please avoid using the search feature and navigation bar in PR previews!

@joelkandiah
Copy link

joelkandiah commented Oct 1, 2025

It may make sense to specify exactly how thinning works in a similar way to the table in Thinning and warmup. This is because the thinning only applies to steps after discard_initial. This partiallye exists already, however doesn't note the interaction with num_warmup.

This is relevant as if I specify sample(..., N = 100, ...; discard_initial = 100, thinning = 10, num_warmup = 500 ) the code should call step_warmup 500 times and then step 591 times, which makes sense but may be unintuitive for first time users (i.e. N only starts counting from after discard initial, and N-1 multiplied by thinning and counts from 1 rather than 0, but num_warmup counts from the first discard iteration).

```
:::

## Thinning and warmup

Choose a reason for hiding this comment

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

We should probably note that kwargs are forwarded to sample calls, which means that individual samplers may provide additional options to sample. This is relevant for HMC(...) where it expects nadapts and discard_adapt to be passed to sample (rather than the sampler).

This also explains why num_warmup is only provided for some samplers.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeahhh, I wasn't sure how to go about explaining this. HMC and NUTS just do weird stuff. I'll think about how to write it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I looked into this a bit more. This is all a bit of a mess because nadapts is actually also a sampler parameter, but then it's fed through into the kwargs for sample. This is probably because the code for NUTS is quite old and hasn't kept up with interface changes.

My feeling right now is that we should remove nadapts from the sampler constructor and just make it use num_warmup instead to signify the number of adaptations. In that case, I would argue it's not worth documenting it right now — would that be fine?

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.

Chain save/resume
2 participants