[Merged by Bors] - Moved LogDensityFunction from Turing to DPPL #447
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| Return instance similar to `vi` but with `vns` set to values from `vals`. | ||
| """ | ||
| function update_values!!(vi::AbstractVarInfo, vals::NamedTuple, vns) | ||
| for vn in vns |
There was a problem hiding this comment.
Made me wonder if there are any advantages/disadvantages compared with a more functional approach using e.g. foldl. Feel free to ignore though, I'm just curious but don't think it matters for this PR.
There was a problem hiding this comment.
Haha yeah when I moved this I was thinking "we should probably do this using recursion or a reduce but I'll leave it for now". I think at some point it might be worth including this in DPPL proper rather than in TestUtils, and then I think we should look into this 👍
src/test_utils.jl
Outdated
| """ | ||
| function test_values(vi::AbstractVarInfo, vals::NamedTuple, vns) | ||
| for vn in vns | ||
| @test vi[vn] == get(vals, vn) |
There was a problem hiding this comment.
Would it be useful to check approximate rquality as well? By allowing e.g. to specify a test function (with the default being ==)?
There was a problem hiding this comment.
Ah good point! I'll make it a kwarg.
…DynamicPPL.jl into torfjelde/logdensityfunction
Co-authored-by: David Widmann <[email protected]>
…DynamicPPL.jl into torfjelde/logdensityfunction
|
Wanna have another look @devmotion ? |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: David Widmann <[email protected]>
Co-authored-by: David Widmann <[email protected]>
|
Good to go? |
devmotion
left a comment
devmotion
left a comment
There was a problem hiding this comment.
Looks good to me, feel free to merge once the minor remaining comments are resolved.
src/test_utils.jl
Outdated
| end | ||
|
|
||
| """ | ||
| DynamicPPL.TestUtils.setup_varinfos(model::Model, example_values::NamedTuple, varnames) |
There was a problem hiding this comment.
Any particular reason for why the modules are included in the signature here but in the docstrings above?
There was a problem hiding this comment.
Whoos, no that's just a misplaced replace! Will fix.
Co-authored-by: David Widmann <[email protected]>
|
bors r+ |
|
Pull request successfully merged into master. Build succeeded: |
bors
bot
changed the title
Related to TuringLang/Turing.jl#1936