search

TuringLang / AbstractMCMC.jl

Abstract types and interfaces for Markov chain Monte Carlo methods
https://turinglang.org/AbstractMCMC.jl
MIT License
78 stars 18 forks source link

Fix method ambiguity issues #123

Closed devmotion closed 1 year ago

devmotion commented 1 year ago

Alternative to #120 which keeps the current functionality.

@torfjelde Do you have a MWE of the ambiguity issues? Or can confirm that this PR would fix the issues you ran into?

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04 :tada:

Comparison is base (e149f9f) 97.33% compared to head (ffb7b43) 97.37%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #123 +/- ## ========================================== + Coverage 97.33% 97.37% +0.04% ========================================== Files 8 8 Lines 300 305 +5 ========================================== + Hits 292 297 +5 Misses 8 8 ``` | [Impacted Files](https://app.codecov.io/gh/TuringLang/AbstractMCMC.jl/pull/123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TuringLang) | Coverage Δ | | |---|---|---| | [src/interface.jl](https://app.codecov.io/gh/TuringLang/AbstractMCMC.jl/pull/123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TuringLang#diff-c3JjL2ludGVyZmFjZS5qbA==) | `95.45% <100.00%> (+0.45%)` | :arrow_up: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/TuringLang/AbstractMCMC.jl/pull/123/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TuringLang)

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

torfjelde commented 1 year ago

I'll have to dig out those examples; it's been a while. But will try to get around to it sometime today.

devmotion commented 1 year ago

Were you able to find (and run?) the examples?

torfjelde commented 1 year ago

No, sorry haven't done it yet. The original examples in #120 are from MCMCTempering.jl which I haven't done in a while. This PR most certainly helps with the overall issue we've discussed, whether or not it actuallly solves the particular ambiguity I ran into.

devmotion commented 1 year ago

Should we merge then maybe merge the PR and revisit #120 (or some variant) if problems show up again?

torfjelde commented 1 year ago

I'm happy with that:) I have NeurIPS reviews over the next few weeks, but once that's done I'll revisit MCMCTempering, and so might be able to provide some cases in #120 then :+1: But for now, let's just merge this.