Skip to content

Add nfidd_sample function to speed up course inference#457

Merged
sbfnk merged 12 commits intomainfrom
feature/speed-up-inference
Jun 20, 2025
Merged

Add nfidd_sample function to speed up course inference#457
sbfnk merged 12 commits intomainfrom
feature/speed-up-inference

Conversation

@seabbs
Copy link
Copy Markdown
Contributor

@seabbs seabbs commented Jun 20, 2025

Summary

  • Add nfidd_sample() function with optimized defaults for course speed
  • Reduce default MCMC iterations from 1000 to 500 for both warmup and sampling
  • Set parallel_chains = 4 by default to utilize multiple cores
  • Update course materials to use the new function throughout sessions

Changes

  • R/nfidd-stan-tools.r: New nfidd_sample() function with course-optimized defaults
  • NAMESPACE: Export new function
  • man/nfidd_sample.Rd: Complete documentation
  • sessions/*.qmd: Update all sessions to use nfidd_sample() instead of direct model$sample()

Rationale

The current Stan models can be slow for course exercises, particularly when students are running multiple models or working on slower machines. This function provides:

  1. Faster iteration: 500 iterations (vs 1000 default) maintains statistical validity while halving runtime
  2. Parallel processing: 4 chains by default utilizes modern multi-core systems
  3. Flexibility: All cmdstanr arguments still accessible via ... for experimentation
  4. Consistency: Standardized settings across all course materials

Testing Plan

  • Function compiles and exports correctly
  • Verify inference quality on deployed site (as noted by maintainer)
  • Monitor student feedback on model fitting speed during course delivery

Closes #220

🤖 Generated with Claude Code

seabbs and others added 3 commits June 20, 2025 10:50
- Reduce default iterations from 1000 to 500 for both warmup and sampling
- Set parallel_chains to 4 by default
- Update course materials to use new function
- Add comprehensive documentation with @inheritParams
- Maintain full cmdstanr compatibility via ... parameter

Addresses #220 by providing faster inference for course exercises
while maintaining statistical validity for educational purposes.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Fix malformed YAML header that was preventing proper rendering
of the R estimation and renewal equation session.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Remove stray ::: on line 60 that was causing markdown parsing issues.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@seabbs
Copy link
Copy Markdown
Contributor Author

seabbs commented Jun 20, 2025

Found and fixed an unpaired ::: in the forecasting-models session that was causing markdown parsing issues. The stray ::: on line 60 has been removed.

@sbfnk
Copy link
Copy Markdown
Contributor

sbfnk commented Jun 20, 2025

Do we also want to apply this to the data generation in data-raw/generate-example-forecasts.r?

@seabbs
Copy link
Copy Markdown
Contributor Author

seabbs commented Jun 20, 2025

I didn't for now as I didn't want to rerun but but if we do need to then yes

@seabbs
Copy link
Copy Markdown
Contributor Author

seabbs commented Jun 20, 2025

I skim read the whole course and from what is in the html I saw nothing concerning i.e the figures and the printed summary. As we suppress warnings though there is a chance that some of the models haven't converged but from the site I saw no evidence of that.

Copy link
Copy Markdown
Contributor

@sbfnk sbfnk left a comment

Choose a reason for hiding this comment

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

I can see benefits for sticking with the original syntax and just writing the smaller number of samples explicitly over hiding the cmdstanr command but I don't feel strongly.

Comment thread R/nfidd-stan-tools.r Outdated
Comment thread sessions/delay-distributions.qmd Outdated
Comment thread sessions/forecasting-models.qmd Outdated
Comment thread sessions/R-Stan-and-statistical-concepts.qmd Outdated
Comment thread sessions/R-Stan-and-statistical-concepts.qmd
seabbs and others added 4 commits June 20, 2025 12:15
Co-authored-by: Sebastian Funk <sebastian.funk@lshtm.ac.uk>
Co-authored-by: Sebastian Funk <sebastian.funk@lshtm.ac.uk>
Co-authored-by: Sebastian Funk <sebastian.funk@lshtm.ac.uk>
Co-authored-by: Sebastian Funk <sebastian.funk@lshtm.ac.uk>
@seabbs
Copy link
Copy Markdown
Contributor Author

seabbs commented Jun 20, 2025

I can see benefits for sticking with the original syntax and just writing the smaller number of samples explicitly over hiding the cmdstanr command but I don't feel strongly.

I didn't do this as I thought we wanted to tell people once and then not make them worry about it as imo its not super important to the learning objectives. It also makes it easier to change if we need to going forward.

@seabbs seabbs requested a review from sbfnk June 20, 2025 11:19
@seabbs seabbs enabled auto-merge June 20, 2025 11:20
@seabbs seabbs added this pull request to the merge queue Jun 20, 2025
Comment thread sessions/delay-distributions.qmd Outdated
@sbfnk sbfnk removed this pull request from the merge queue due to a manual request Jun 20, 2025
@sbfnk sbfnk enabled auto-merge June 20, 2025 11:51
@sbfnk sbfnk added this pull request to the merge queue Jun 20, 2025
Merged via the queue into main with commit 2181b56 Jun 20, 2025
8 checks passed
@sbfnk sbfnk deleted the feature/speed-up-inference branch June 20, 2025 12:29
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.

review opportunities for speeding up some of the models run during sessions

2 participants