TuringLang
Remove `default_varinfo` function
default_varinfo isn't used anywhere else in TuringLang and this PR helps to remove one level of indirection.
Also updated the comment to help provide some context.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 79.20%. Comparing base (
54691bf) to head (23358b4). Report is 2 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #695 +/- ##
==========================================
- Coverage 79.22% 79.20% -0.03%
==========================================
Files 30 30
Lines 4212 4207 -5
==========================================
- Hits 3337 3332 -5
Misses 875 875
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Pull Request Test Coverage Report for Build 11394594173
Details
- 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
- 73 unchanged lines in 17 files lost coverage.
- Overall coverage decreased (-0.02%) to 79.599%
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| src/simple_varinfo.jl | 1 | 89.18% |
| src/sampler.jl | 1 | 92.0% |
| ext/DynamicPPLEnzymeCoreExt.jl | 1 | 0.0% |
| src/contexts.jl | 3 | 75.76% |
| src/context_implementations.jl | 3 | 63.67% |
| src/test_utils.jl | 3 | 85.31% |
| src/model_utils.jl | 3 | 19.64% |
| src/varnamedvector.jl | 3 | 89.42% |
| src/distribution_wrappers.jl | 4 | 44.12% |
| ext/DynamicPPLForwardDiffExt.jl | 4 | 66.67% |
| <!-- | Total: | 73 |
| Totals | |
|---|---|
| Change from base Build 11381380435: | -0.02% |
| Covered Lines: | 3332 |
| Relevant Lines: | 4186 |
💛 - Coveralls
So this was deliberately introduced at some point exactly to allow using other AbstractVarInfo implementations for a given sampler by overloading this method. It's true that it's not used anywhere, but I have personally used this method in the past to use SimpleVarInfo for a specific model because it lead to performance improvements. Sooo I'm somewhat reluctant to remove it 🤷 But yeah, up for discussion.
@torfjelde, I'm sorry, you stumbled across a PRs which is definitely not intended for review yet 😅 I've been trying to see how much of the codebase I can strip out without CI failing (hence a few force pushes). I will definitely consult for opinions before asking for proper reviews!
Eh, I can't remember what I was trying to do anymore. :// It was something with the experimental Gibbs sampler and trying to trace where things were going. Anyway, I figure it can just be closed.