Skip to content
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

Remove samplers from VarInfo - Selectors and GIDs #808

Merged
merged 4 commits into from
Feb 17, 2025

Conversation

mhauru
Copy link
Member

@mhauru mhauru commented Feb 13, 2025

Continuing the decoupling of samplers and VarInfo, this one removes the Gibbs ID field and everything related to Selectors. That is, the mechanism by which VarInfo associated different variables with different Gibbs components.

This might be the last one, but I need to do a sweep and try running the Turing.jl test suite against this version of DPPL to see that I haven't missed anything.

Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 86.32%. Comparing base (29f3760) to head (7ab6994).
Report is 1 commits behind head on release-0.35.

Files with missing lines Patch % Lines
src/context_implementations.jl 83.33% 1 Missing ⚠️
src/varinfo.jl 87.50% 1 Missing ⚠️
src/varnamedvector.jl 50.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release-0.35     #808      +/-   ##
================================================
+ Coverage         85.78%   86.32%   +0.54%     
================================================
  Files                36       35       -1     
  Lines              4207     4162      -45     
================================================
- Hits               3609     3593      -16     
+ Misses              598      569      -29     

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

@mhauru mhauru closed this Feb 13, 2025
@mhauru mhauru reopened this Feb 13, 2025
@mhauru mhauru marked this pull request as ready for review February 13, 2025 14:41
@mhauru mhauru requested review from penelopeysm and sunxd3 and removed request for penelopeysm February 13, 2025 14:41
@sunxd3
Copy link
Member

sunxd3 commented Feb 13, 2025

thanks Markus, I'll take a look -- but give me a bit of time to look over and think a bit, will try to get it done tomorrow morning

Copy link
Member

@sunxd3 sunxd3 left a comment

Choose a reason for hiding this comment

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

I do not have a very good intuition about the implications, so can't comment beyond surface level.

The changes are very well contained. I tried my best to look for potential leftovers and found none.

So it looks good to me! Great effort.

@sunxd3
Copy link
Member

sunxd3 commented Feb 14, 2025

I assume the test errors are of the same sources as #804 (comment)?

@coveralls
Copy link

Pull Request Test Coverage Report for Build 13367790756

Details

  • 15 of 20 (75.0%) changed or added relevant lines in 5 files are covered.
  • 26 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-12.3%) to 73.612%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/context_implementations.jl 5 6 83.33%
src/varinfo.jl 7 8 87.5%
src/varnamedvector.jl 1 2 50.0%
src/threadsafe.jl 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
src/varinfo.jl 3 83.37%
src/model.jl 5 80.0%
src/threadsafe.jl 18 51.43%
Totals Coverage Status
Change from base Build 13345435491: -12.3%
Covered Lines: 3049
Relevant Lines: 4142

💛 - Coveralls

@TuringLang TuringLang deleted a comment from coveralls Feb 17, 2025
@TuringLang TuringLang deleted a comment from coveralls Feb 17, 2025
@TuringLang TuringLang deleted a comment from coveralls Feb 17, 2025
@TuringLang TuringLang deleted a comment from coveralls Feb 17, 2025
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

@TuringLang TuringLang deleted a comment from coveralls Feb 17, 2025
@mhauru mhauru merged commit 4b9665a into release-0.35 Feb 17, 2025
17 of 18 checks passed
@mhauru mhauru deleted the mhauru/remove-selectors-gids branch February 17, 2025 18:20
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.

4 participants