Skip to content

Improvements to DynamicPPLBenchmarks #346

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

Merged
merged 49 commits into from
Mar 12, 2025
Merged

Improvements to DynamicPPLBenchmarks #346

merged 49 commits into from
Mar 12, 2025

Conversation

torfjelde
Copy link
Member

@torfjelde torfjelde commented Dec 3, 2021

Produces results such as can be seen here.

@torfjelde torfjelde marked this pull request as draft December 3, 2021 00:43
@yebai
Copy link
Member

yebai commented Dec 16, 2021

This might be helpful for running benchmarks via CI - https://github.com/tkf/BenchmarkCI.jl

@yebai
Copy link
Member

yebai commented Aug 29, 2022

@torfjelde should we improve this PR by incorporating TuringBenchmarks ? Alternatively, we can move all benchmarking code here into TuringBenchmarks . I am happy with both cases, but ideally, these benchmarking utilities should live in only one place to minimise confusion.

Also, https://github.com/TuringLang/TuringExamples contains some very old benchmarking code.

cc @xukai92 @devmotion

@coveralls
Copy link

coveralls commented Feb 2, 2023

Pull Request Test Coverage Report for Build 13810813889

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 27 unchanged lines in 11 files lost coverage.
  • Overall coverage remained the same at 84.658%

Files with Coverage Reduction New Missed Lines %
ext/DynamicPPLForwardDiffExt.jl 1 63.64%
src/sampler.jl 1 89.06%
src/simple_varinfo.jl 1 75.58%
src/varnamedvector.jl 1 90.1%
src/contexts.jl 2 30.21%
src/logdensityfunction.jl 2 52.27%
src/model.jl 2 80.0%
src/utils.jl 2 72.61%
src/values_as_in_model.jl 3 59.52%
src/distribution_wrappers.jl 4 27.78%
Totals Coverage Status
Change from base Build 13810591547: 0.0%
Covered Lines: 3239
Relevant Lines: 3826

💛 - Coveralls

@codecov
Copy link

codecov bot commented Feb 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.56%. Comparing base (0b13164) to head (dae1be7).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     TuringLang/DynamicPPL.jl#346   +/-   ##
=======================================
  Coverage   84.56%   84.56%           
=======================================
  Files          34       34           
  Lines        3830     3830           
=======================================
  Hits         3239     3239           
  Misses        591      591           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yebai
Copy link
Member

yebai commented Mar 10, 2025

It would also break reproducibility for anything that hasn't committed their Manifest.toml to version control, e.g. any notebook that starts with something like import Pkg; Pkg.add("TuringBenchmarking") would break.

I am not aware of wide use of TuringBenchmarking yet but that is a possibility.

It is okay to duplicate / re-implement the TuringBenchmarking code inside DynamicPPL and then depreciate it without yanking it.

@mhauru
Copy link
Member

mhauru commented Mar 10, 2025

Thanks @shravanngoswamii, looks good, nice touch on making the commit hash a link too.

I pushed a few commits that

  1. Add the dimension @yebai was asking for.
  2. Use Mooncake for most of the benchmarks, rather than ReverseDiff.
  3. Fix a type stability issue with the smorgasbord model
  4. Add an @info print to track progress.

Anyone object to merging this?

Copy link
Member

@yebai yebai left a comment

Choose a reason for hiding this comment

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

Thanks @mhauru @shravanngoswamii and @torfjelde. Great that this is finally ready to merge (it only takes a few years)!

@penelopeysm
Copy link
Member

I'd like to review, if that's ok

@penelopeysm
Copy link
Member

I hope nothing too onerous, and I promise these are my last comments

@mhauru mhauru requested a review from penelopeysm March 11, 2025 16:41
Copy link
Member

@penelopeysm penelopeysm left a comment

Choose a reason for hiding this comment

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

LGTM! Well done to everyone on this nice piece of work.

Copy link
Member

@mhauru mhauru left a comment

Choose a reason for hiding this comment

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

@shravanngoswamii, do you want to check that you're happy as well, and hit merge if so?

@shravanngoswamii
Copy link
Member

shravanngoswamii commented Mar 12, 2025

@mhauru, Everything looks good to me! Can you hit the merge button, I am not authorized to push to main branch in this repo!

@mhauru mhauru enabled auto-merge March 12, 2025 11:31
@mhauru mhauru added this pull request to the merge queue Mar 12, 2025
Merged via the queue into main with commit 5c89efc Mar 12, 2025
18 checks passed
@mhauru mhauru deleted the tor/benchmark-update branch March 12, 2025 13:31
@shravanngoswamii
Copy link
Member

Finally, It's merged! Thank you, @yebai @mhauru @penelopeysm and @torfjelde!

@mhauru
Copy link
Member

mhauru commented Mar 12, 2025

Thanks @shravanngoswamii, good work!

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.

7 participants