Remove samplers from VarInfo - indexing #793

mhauru · 2025-01-30T14:19:56Z

Part two of removing samplers from the internals of VarInfo, this time addressing getindex, setindex, eltype, keys, and functions related to them. Also picking up small bits that should have been done in the previous PR, and fixing at least one small, unrelated bug.

This one is hopefully more straight-forward, in that it mostly just removes things, very little new code is introduced.

…tors-linking

Co-authored-by: Xianda Sun <[email protected]>

Co-authored-by: Penelope Yong <[email protected]>

…-selectors-linking

…-selectors-indexing

…ace, update HISTORY.md

codecov · 2025-01-30T16:02:33Z

Codecov Report

Attention: Patch coverage is 71.69811% with 15 lines in your changes missing coverage. Please review.

Please upload report for BASE (release-0.35@c5f2f7a). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/varinfo.jl	58.82%	7 Missing ⚠️
src/compiler.jl	80.00%	3 Missing ⚠️
src/sampler.jl	81.81%	2 Missing ⚠️
src/abstract_varinfo.jl	66.66%	1 Missing ⚠️
src/simple_varinfo.jl	50.00%	1 Missing ⚠️
src/threadsafe.jl	0.00%	1 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##             release-0.35     #793   +/-   ##
===============================================
  Coverage                ?   85.78%           
===============================================
  Files                   ?       36           
  Lines                   ?     4207           
  Branches                ?        0           
===============================================
  Hits                    ?     3609           
  Misses                  ?      598           
  Partials                ?        0

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

coveralls · 2025-01-30T16:04:16Z

Pull Request Test Coverage Report for Build 13055998519

Details

38 of 53 (71.7%) changed or added relevant lines in 8 files are covered.
42 unchanged lines in 5 files lost coverage.
Overall coverage decreased (-12.2%) to 73.203%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/abstract_varinfo.jl	2	3	66.67%
src/simple_varinfo.jl	1	2	50.0%
src/threadsafe.jl	0	1	0.0%
src/sampler.jl	9	11	81.82%
src/compiler.jl	12	15	80.0%
src/varinfo.jl	10	17	58.82%

Files with Coverage Reduction	New Missed Lines	%
src/contexts.jl	1	31.41%
src/sampler.jl	1	91.18%
src/model.jl	5	80.0%
src/varinfo.jl	14	82.62%
src/threadsafe.jl	21	50.47%

Totals
Change from base Build 13052659735:	-12.2%
Covered Lines:	3065
Relevant Lines:	4187

💛 - Coveralls

coveralls · 2025-01-30T16:12:28Z

Pull Request Test Coverage Report for Build 13055998519

Details

38 of 53 (71.7%) changed or added relevant lines in 8 files are covered.
42 unchanged lines in 5 files lost coverage.
Overall coverage increased (+0.5%) to 85.867%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/abstract_varinfo.jl	2	3	66.67%
src/simple_varinfo.jl	1	2	50.0%
src/threadsafe.jl	0	1	0.0%
src/sampler.jl	9	11	81.82%
src/compiler.jl	12	15	80.0%
src/varinfo.jl	10	17	58.82%

Files with Coverage Reduction	New Missed Lines	%
src/contexts.jl	1	31.41%
src/sampler.jl	1	91.18%
src/model.jl	5	80.0%
src/varinfo.jl	14	82.62%
src/threadsafe.jl	21	50.47%

Totals
Change from base Build 13052659735:	0.5%
Covered Lines:	3609
Relevant Lines:	4203

💛 - Coveralls

coveralls · 2025-01-30T16:12:28Z

Pull Request Test Coverage Report for Build 13116927069

Details

38 of 53 (71.7%) changed or added relevant lines in 8 files are covered.
42 unchanged lines in 5 files lost coverage.
Overall coverage increased (+0.5%) to 85.867%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/abstract_varinfo.jl	2	3	66.67%
src/simple_varinfo.jl	1	2	50.0%
src/threadsafe.jl	0	1	0.0%
src/sampler.jl	9	11	81.82%
src/compiler.jl	12	15	80.0%
src/varinfo.jl	10	17	58.82%

Files with Coverage Reduction	New Missed Lines	%
src/contexts.jl	1	31.41%
src/sampler.jl	1	91.18%
src/model.jl	5	80.0%
src/varinfo.jl	14	82.62%
src/threadsafe.jl	21	50.47%

Totals
Change from base Build 13052659735:	0.5%
Covered Lines:	3609
Relevant Lines:	4203

💛 - Coveralls

penelopeysm

I've only skimmed, so these are general impressions. I'll double check the deleted code in more detail at a later date, but at first glance it all seemed fine (I guess that's a literal LGTM).

src/compiler.jl

src/varinfo.jl

sunxd3

It looks good to me!

src/compiler.jl

Co-authored-by: Penelope Yong <[email protected]>

penelopeysm

Finally got to take a second look at the removed code - it all looks great!

mhauru and others added 30 commits January 16, 2025 16:14

Remove selector stuff from varinfo tests

4dc2a72

Implement link and invlink for varnames rather than samplers

9b492a3

Replace set_retained_vns_del_by_spl! with set_retained_vns_del!

b508f08

Make linking tests more extensive

b8880d1

Remove sampler indexing from link methods (but not invlink)

99a8490

Remove indexing by samplers from invlink

4a79b1f

Merge remote-tracking branch 'origin/master' into mhauru/remove-selec…

26a1901

…tors-linking

Work towards removing sampler indexing with StaticTransformation

090608b

Fix invlink/link for TypedVarInfo and StaticTransformation

4749853

Fix a test in models.jl

e960679

Move some functions to utils.jl, add tests and docstrings

d507a53

Fix a docstring typo

41150b5

Merge branch 'release-0.35' into mhauru/remove-selectors-linking

836fb13

Various simplification to link/invlink

45d1f13

Improve a docstring

98915c2

Style improvements

f05068d

Fix broken link/invlink dispatch cascade for VectorVarInfo

bc4c420

Fix some more broken dispatch cascades

71980ba

Apply suggestions from code review

45562a9

Co-authored-by: Xianda Sun <[email protected]>

Remove comments that messed with docstrings

db5b835

Apply suggestions from code review

f99effe

Co-authored-by: Penelope Yong <[email protected]>

Fix issues surfaced in code review

56194cd

Simplify link/invlink arguments

c187c49

Fix a bug in unflatten VarNamedVector

86b25c5

Rename VarNameCollection -> VarNameTuple

2a6c1bc

Remove test of a removed varname_namedtuple method

853f47e

Apply suggestions from code review

ed80328

Co-authored-by: Penelope Yong <[email protected]>

Respond to review feedback

d996d0c

Remove _default_sampler and a dead argument of maybe_invlink_before_eval

2083148

Fix a typo in a comment

39fa647

mhauru added 14 commits January 30, 2025 11:43

Merge remote-tracking branch 'origin/release-0.35' into mhauru/remove…

9df364f

…-selectors-linking

Add HISTORY entry, fix one set_retained_vns_del! method

2c73de5

Merge remote-tracking branch 'origin/release-0.35' into mhauru/remove…

49604e1

…-selectors-linking

Remove some VarInfo getindex with samplers stuff

1c50d0c

Remove some index setting with samplers

b919fe4

Remove more sampler indexing

fc0e064

Remove unflatten with samplers

5fbe016

Clean up some setindex stuff

cb5c79d

Merge remote-tracking branch 'origin/release-0.35' into mhauru/remove…

fa696d3

…-selectors-indexing

Remove a bunch of varinfo.jl internal functions that used samplers/sp…

414f58e

…ace, update HISTORY.md

Fix HISTORY.md

800b91a

Miscalleanous small fixes

e65777e

Fix a bug in VarInfo constructor

8fcc289

Fix getparams(::LogDensityFunction)

c59aafe

mhauru marked this pull request as ready for review January 30, 2025 16:14

mhauru requested review from penelopeysm and sunxd3 January 30, 2025 16:14

penelopeysm reviewed Jan 30, 2025

View reviewed changes

src/compiler.jl Outdated Show resolved Hide resolved

src/varinfo.jl Show resolved Hide resolved

src/varinfo.jl Show resolved Hide resolved

sunxd3 reviewed Jan 30, 2025

View reviewed changes

src/compiler.jl Show resolved Hide resolved

Apply suggestions from code review

934fb79

Co-authored-by: Penelope Yong <[email protected]>

penelopeysm mentioned this pull request Feb 3, 2025

Inline (and more carefully use) islinked inside initialize_parameters!! #797

Open

penelopeysm approved these changes Feb 3, 2025

View reviewed changes

mhauru merged commit 1366440 into release-0.35 Feb 5, 2025
16 checks passed

mhauru deleted the mhauru/remove-selectors-indexing branch February 5, 2025 11:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove samplers from VarInfo - indexing #793

Remove samplers from VarInfo - indexing #793

mhauru commented Jan 30, 2025 •

edited

Loading

codecov bot commented Jan 30, 2025 •

edited

Loading

coveralls commented Jan 30, 2025 •

edited

Loading

coveralls commented Jan 30, 2025

coveralls commented Jan 30, 2025 •

edited

Loading

penelopeysm left a comment

sunxd3 left a comment

penelopeysm left a comment

Remove samplers from VarInfo - indexing #793

Remove samplers from VarInfo - indexing #793

Conversation

mhauru commented Jan 30, 2025 • edited Loading

codecov bot commented Jan 30, 2025 • edited Loading

Codecov Report

coveralls commented Jan 30, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13055998519

Details

💛 - Coveralls

coveralls commented Jan 30, 2025

Pull Request Test Coverage Report for Build 13055998519

Details

💛 - Coveralls

coveralls commented Jan 30, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13116927069

Details

💛 - Coveralls

penelopeysm left a comment

Choose a reason for hiding this comment

sunxd3 left a comment

Choose a reason for hiding this comment

penelopeysm left a comment

Choose a reason for hiding this comment

mhauru commented Jan 30, 2025 •

edited

Loading

codecov bot commented Jan 30, 2025 •

edited

Loading

coveralls commented Jan 30, 2025 •

edited

Loading

coveralls commented Jan 30, 2025 •

edited

Loading