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

Unify transition also in external samplers #2030

Merged
merged 12 commits into from
Jul 10, 2023
Merged

Conversation

JaimeRZP
Copy link
Member

@JaimeRZP JaimeRZP commented Jul 5, 2023

We no longer need TuringTransition

@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

Pull Request Test Coverage Report for Build 5494388244

  • 0 of 5 (0.0%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 0.0%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/contrib/inference/abstractmcmc.jl 0 1 0.0%
src/inference/Inference.jl 0 4 0.0%
Files with Coverage Reduction New Missed Lines %
src/contrib/inference/abstractmcmc.jl 2 0%
Totals Coverage Status
Change from base Build 5462077783: 0.0%
Covered Lines: 0
Relevant Lines: 1444

💛 - Coveralls

@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (a67d0ce) 0.00% compared to head (911a55f) 0.00%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #2030   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files          21      22    +1     
  Lines        1419    1444   +25     
======================================
- Misses       1419    1444   +25     
Impacted Files Coverage Δ
src/Turing.jl 0.00% <ø> (ø)
src/contrib/inference/abstractmcmc.jl 0.00% <0.00%> (ø)
src/inference/Inference.jl 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@JaimeRZP JaimeRZP requested review from torfjelde and devmotion and removed request for torfjelde July 6, 2023 08:11
@JaimeRZP JaimeRZP marked this pull request as ready for review July 6, 2023 10:44
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 @JaimeRZP - it looks good!

src/inference/Inference.jl Outdated Show resolved Hide resolved
src/inference/Inference.jl Outdated Show resolved Hide resolved
src/inference/Inference.jl Outdated Show resolved Hide resolved
src/inference/Inference.jl Outdated Show resolved Hide resolved
src/inference/Inference.jl Outdated Show resolved Hide resolved
@torfjelde
Copy link
Member

torfjelde commented Jul 7, 2023

@JaimeRZP are you not also changing HMCTransition in this PR? Isn't that redundant now?

EDIT: Nvm, I completely forgot that was already done 🤦

@yebai yebai merged commit a1bf714 into master Jul 10, 2023
@yebai yebai deleted the Transition_in_external_sampler branch July 10, 2023 13:41
@yebai
Copy link
Member

yebai commented Jul 10, 2023

Nice improvements -- thanks, @JaimeRZP!

@@ -39,7 +39,6 @@ Tracker = "9f7883ad-71c0-57eb-9f7f-b5c9e6d3789c"
[compat]
AbstractMCMC = "4"
AdvancedHMC = "0.3.0, 0.4"
AdvancedMH = "0.6.8, 0.7"
Copy link
Member

Choose a reason for hiding this comment

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

Just noticed now, seems this was removed by accident?

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