Release 0.35: Remove Selectors #779

mhauru · 2025-01-16T16:14:08Z

This PR will collect all work related to removing Selectors, Gibbs IDs, and indexing VarInfo with samplers. Since this will involve quite a lot of changes, we'll try a new pattern for code review: Changes will be made incrementally, trying to remove parts of the functionality at a time and getting tests to pass in between. Each such incremental change will be its own PR into this branch, release-0.35. Careful code review will be done at that stage. By the end of it this PR will probably be massive, and heavily breaking, but that can all then go into master with only a light, final review.

Issue(s) closed

Closes #786

Unblock #771: the dependency on LogDensityProblemAD has been replaced by LogDensityFunction.

codecov · 2025-01-16T16:59:37Z

Codecov Report

Attention: Patch coverage is 84.72622% with 53 lines in your changes missing coverage. Please review.

Project coverage is 84.60%. Comparing base (5c0cebe) to head (90c7b26).

Files with missing lines	Patch %	Lines
src/varinfo.jl	83.33%	25 Missing ⚠️
src/threadsafe.jl	47.61%	11 Missing ⚠️
src/abstract_varinfo.jl	81.48%	5 Missing ⚠️
ext/DynamicPPLForwardDiffExt.jl	70.00%	3 Missing ⚠️
src/compiler.jl	88.88%	3 Missing ⚠️
src/sampler.jl	81.81%	2 Missing ⚠️
src/contexts.jl	85.71%	1 Missing ⚠️
src/logdensityfunction.jl	97.61%	1 Missing ⚠️
src/simple_varinfo.jl	75.00%	1 Missing ⚠️
src/varnamedvector.jl	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #779      +/-   ##
==========================================
- Coverage   86.17%   84.60%   -1.58%     
==========================================
  Files          36       34       -2     
  Lines        4305     3832     -473     
==========================================
- Hits         3710     3242     -468     
+ Misses        595      590       -5

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

coveralls · 2025-01-23T13:10:55Z

Pull Request Test Coverage Report for Build 13530836280

Details

269 of 346 (77.75%) changed or added relevant lines in 15 files are covered.
103 unchanged lines in 15 files lost coverage.
Overall coverage decreased (-1.6%) to 84.692%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/simple_varinfo.jl	3	4	75.0%
src/varnamedvector.jl	1	2	50.0%
src/sampler.jl	9	11	81.82%
src/compiler.jl	24	27	88.89%
src/contexts.jl	4	7	57.14%
ext/DynamicPPLForwardDiffExt.jl	6	10	60.0%
src/abstract_varinfo.jl	22	27	81.48%
src/threadsafe.jl	8	21	38.1%
src/logdensityfunction.jl	21	41	51.22%
src/varinfo.jl	125	150	83.33%

Files with Coverage Reduction	New Missed Lines	%
src/logdensityfunction.jl	1	52.27%
src/varnamedvector.jl	1	90.12%
src/context_implementations.jl	2	81.25%
src/debug_utils.jl	2	0.0%
src/model.jl	2	78.33%
src/threadsafe.jl	2	51.43%
src/contexts.jl	4	30.21%
src/sampler.jl	4	89.06%
src/utils.jl	7	72.61%
src/extract_priors.jl	8	71.43%

Totals
Change from base Build 13530825014:	-1.6%
Covered Lines:	3242
Relevant Lines:	3828

💛 - Coveralls

* Reverse order of prefixing * Simplify generated function (the non-generated path wasn't being used) * Expand on submodel behaviour in changelog

* Remove selector stuff from varinfo tests * Implement link and invlink for varnames rather than samplers * Replace set_retained_vns_del_by_spl! with set_retained_vns_del! * Make linking tests more extensive * Remove sampler indexing from link methods (but not invlink) * Remove indexing by samplers from invlink * Work towards removing sampler indexing with StaticTransformation * Fix invlink/link for TypedVarInfo and StaticTransformation * Fix a test in models.jl * Move some functions to utils.jl, add tests and docstrings * Fix a docstring typo * Various simplification to link/invlink * Improve a docstring * Style improvements * Fix broken link/invlink dispatch cascade for VectorVarInfo * Fix some more broken dispatch cascades * Apply suggestions from code review Co-authored-by: Xianda Sun <[email protected]> * Remove comments that messed with docstrings * Apply suggestions from code review Co-authored-by: Penelope Yong <[email protected]> * Fix issues surfaced in code review * Simplify link/invlink arguments * Fix a bug in unflatten VarNamedVector * Rename VarNameCollection -> VarNameTuple * Remove test of a removed varname_namedtuple method * Apply suggestions from code review Co-authored-by: Penelope Yong <[email protected]> * Respond to review feedback * Remove _default_sampler and a dead argument of maybe_invlink_before_eval * Fix a typo in a comment * Add HISTORY entry, fix one set_retained_vns_del! method --------- Co-authored-by: Xianda Sun <[email protected]> Co-authored-by: Penelope Yong <[email protected]>

* Remove selector stuff from varinfo tests * Implement link and invlink for varnames rather than samplers * Replace set_retained_vns_del_by_spl! with set_retained_vns_del! * Make linking tests more extensive * Remove sampler indexing from link methods (but not invlink) * Remove indexing by samplers from invlink * Work towards removing sampler indexing with StaticTransformation * Fix invlink/link for TypedVarInfo and StaticTransformation * Fix a test in models.jl * Move some functions to utils.jl, add tests and docstrings * Fix a docstring typo * Various simplification to link/invlink * Improve a docstring * Style improvements * Fix broken link/invlink dispatch cascade for VectorVarInfo * Fix some more broken dispatch cascades * Apply suggestions from code review Co-authored-by: Xianda Sun <[email protected]> * Remove comments that messed with docstrings * Apply suggestions from code review Co-authored-by: Penelope Yong <[email protected]> * Fix issues surfaced in code review * Simplify link/invlink arguments * Fix a bug in unflatten VarNamedVector * Rename VarNameCollection -> VarNameTuple * Remove test of a removed varname_namedtuple method * Apply suggestions from code review Co-authored-by: Penelope Yong <[email protected]> * Respond to review feedback * Remove _default_sampler and a dead argument of maybe_invlink_before_eval * Fix a typo in a comment * Add HISTORY entry, fix one set_retained_vns_del! method * Remove some VarInfo getindex with samplers stuff * Remove some index setting with samplers * Remove more sampler indexing * Remove unflatten with samplers * Clean up some setindex stuff * Remove a bunch of varinfo.jl internal functions that used samplers/space, update HISTORY.md * Fix HISTORY.md * Miscalleanous small fixes * Fix a bug in VarInfo constructor * Fix getparams(::LogDensityFunction) * Apply suggestions from code review Co-authored-by: Penelope Yong <[email protected]> --------- Co-authored-by: Xianda Sun <[email protected]> Co-authored-by: Penelope Yong <[email protected]>

* Remove Selectors and Gibbs IDs * Remove getspace * Remove a dead VNV method --------- Co-authored-by: Xianda Sun <[email protected]>

* Refactor dot_tilde, work in progress * Restrict dot_tilde to univariate dists on the RHS * Remove tests with multivariates or arrays as RHS of .~ * emove dot_tilde pipeline * Fix a .~ bug * Update HISTORY.md * Fix a tiny test bug * Re-enable some SimpleVarInfo tests * Improve changelog entry * Improve error message * Fix trivial typos * Fix pointwise_logdensity test * Remove pointless check_dot_tilde_rhs method * Add tests for old .~ syntax * Bump Mooncake patch version to v0.4.90 * Bump Mooncake to 0.4.95 --------- Co-authored-by: Penelope Yong <[email protected]>

* Remove LogDensityProblemsAD * Implement LogDensityFunctionWithGrad in place of ADgradient * Dynamically decide whether to use closure vs constant * Combine LogDensityFunction{,WithGrad} into one (#811) * Warn if unsupported AD type is used * Update changelog * Update DI compat bound Co-authored-by: Guillaume Dalle <[email protected]> * Don't store with_closure inside LogDensityFunction Co-authored-by: Guillaume Dalle <[email protected]> * setadtype --> LogDensityFunction * Re-add ForwardDiffExt (including tests) * Add more tests for coverage --------- Co-authored-by: Guillaume Dalle <[email protected]>

mhauru · 2025-02-25T17:43:58Z

This PR, and release v0.35 could be ready to go. It ended up involving removing samplers from VarInfo, reimplementing .~, dropping LogDensityProblemsAD, and a few other changes. @penelopeysm, @sunxd3, @willtebbutt, and I have reviewed all the individual PRs that have contributed to this release. Can I ask @yebai to please take a look at the list of breaking changes in HISTORY.md, and let us know if you are happy? Anyone else who's interested as well (@torfjelde?), please speak now if something here troubles you.

There's a companion PR for Turing.jl, TuringLang/Turing.jl#2488, that makes the corresponding changes in that repo, and it's very nearly passing tests, so we seem good on that front.

I would propose doing a non-squash merge of this PR, because the individual commits here already come from substantial PRs themselves, and squashing them into one megacommit would make bisecting a nightmare.

penelopeysm · 2025-02-25T17:48:16Z

Maybe we could squeeze in exporting LogDensityFunction? #816 I reckon it makes sense given the changes to its interface

I can do a quick PR tomorrow

yebai

Thanks @mhauru, @penelopeysm and @sunxd3 -- excellent improvements!

yebai · 2025-02-25T17:47:12Z

.github/workflows/CI.yml

@@ -5,6 +5,7 @@ on:
    branches:
      - master
      - backport-*
+      - release-0.35


Suggested change

- release-0.35

yebai · 2025-02-25T17:50:51Z

docs/src/api.md

@@ -447,10 +440,8 @@ DynamicPPL.Experimental.is_suitable_varinfo

 ```@docs
 tilde_assume
-dot_tilde_assume


It's good to see these gone!

Init release 0.35

a34fb04

Merge remote-tracking branch 'origin/master' into release-0.35

af65bb0

mhauru mentioned this pull request Jan 23, 2025

Remove selector stuff from VarInfo tests and link/invlink #780

Merged

penelopeysm added 3 commits January 28, 2025 12:05

Merge remote-tracking branch 'origin/master' into release-0.35

f5c5fda

Reverse order of prefixing & add changelog (#792)

161edf8

* Reverse order of prefixing * Simplify generated function (the non-generated path wasn't being used) * Expand on submodel behaviour in changelog

Format HISTORY.md

7140f3d

TuringLang deleted a comment from github-actions bot Jan 30, 2025

mhauru and others added 8 commits January 30, 2025 12:46

Merge branch 'master' into release-0.35

7a1051c

Merge branch 'master' into release-0.35

7613dbb

Merge branch 'master' into release-0.35

3521644

Merge remote-tracking branch 'origin/master' into release-0.35

29f3760

Remove samplers from VarInfo - Selectors and GIDs (#808)

4b9665a

* Remove Selectors and Gibbs IDs * Remove getspace * Remove a dead VNV method --------- Co-authored-by: Xianda Sun <[email protected]>

penelopeysm closed this Feb 19, 2025

penelopeysm reopened this Feb 19, 2025

mhauru marked this pull request as ready for review February 25, 2025 17:35

mhauru closed this Feb 25, 2025

mhauru reopened this Feb 25, 2025

mhauru requested a review from yebai February 25, 2025 17:44

yebai approved these changes Feb 25, 2025

View reviewed changes

yebai requested a review from sunxd3 February 25, 2025 17:52

Merge branch 'master' into release-0.35

4bc0d1f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release 0.35: Remove Selectors #779

Release 0.35: Remove Selectors #779

mhauru commented Jan 16, 2025 •

edited by yebai

Loading

codecov bot commented Jan 16, 2025 •

edited

Loading

coveralls commented Jan 23, 2025 •

edited

Loading

mhauru commented Feb 25, 2025 •

edited

Loading

penelopeysm commented Feb 25, 2025 •

edited

Loading

yebai left a comment

yebai Feb 25, 2025

yebai Feb 25, 2025

Release 0.35: Remove Selectors #779

Are you sure you want to change the base?

Release 0.35: Remove Selectors #779

Conversation

mhauru commented Jan 16, 2025 • edited by yebai Loading

Issue(s) closed

codecov bot commented Jan 16, 2025 • edited Loading

Codecov Report

coveralls commented Jan 23, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13530836280

Details

💛 - Coveralls

mhauru commented Feb 25, 2025 • edited Loading

penelopeysm commented Feb 25, 2025 • edited Loading

yebai left a comment

Choose a reason for hiding this comment

yebai Feb 25, 2025

Choose a reason for hiding this comment

yebai Feb 25, 2025

Choose a reason for hiding this comment

mhauru commented Jan 16, 2025 •

edited by yebai

Loading

codecov bot commented Jan 16, 2025 •

edited

Loading

coveralls commented Jan 23, 2025 •

edited

Loading

mhauru commented Feb 25, 2025 •

edited

Loading

penelopeysm commented Feb 25, 2025 •

edited

Loading