Skip to content

Conversation

@devmotion
Copy link
Member

@devmotion devmotion commented Jun 21, 2020

This PR tries to address the inconvenience for users having to know about the internal variable _varinfo and writing Turing.acclogp!(_varinfo, myvalue) to modify the accumulated joint log probability by adding a @addlogprob! macro which allows to simply write @addlogprob!(myvalue).

In particular, it addresses TuringLang/Turing.jl#1332 (comment) (partly, the documentation still has to be updated) and might have avoided the discussion in TuringLang/Turing.jl#1328.

BTW, this macro is quite different from the removed @logprob and @varinfo macros since it is a "proper" Julia macro that is not replaced or touched by the DynamicPPL compiler.

It might seem natural to call it @acclogp! (since it just adds a call to acclogp!) but IMO a more descriptive name (such as @addlogprob!?) might be more intuitive for users.

@devmotion
Copy link
Member Author

The question came up again in the Slack channel yesterday, so I guess users really would like to use this functionality (in whatever way it's possible) and the documentation should mention it explicitly.

Copy link
Contributor

@mohdibntarek mohdibntarek left a comment

Choose a reason for hiding this comment

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

LGTM

@devmotion
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jun 29, 2020
This PR tries to address the inconvenience for users having to know about the internal variable `_varinfo` and writing `Turing.acclogp!(_varinfo, myvalue)` to modify the accumulated joint log probability by adding a `@addlogprob!` macro which allows to simply write `@addlogprob!(myvalue)`.

In particular, it addresses TuringLang/Turing.jl#1332 (comment) (partly, the documentation still has to be updated) and might have avoided the discussion in TuringLang/Turing.jl#1328.

BTW, this macro is quite different from the removed `@logprob` and `@varinfo` macros since it is a "proper" Julia macro that is not replaced or touched by the DynamicPPL compiler.

It might seem natural to call it `@acclogp!` (since it just adds a call to `acclogp!`) but IMO a more descriptive name (such as `@addlogprob!`?) might be more intuitive for users.
@bors bors bot changed the title Add @addlogprob! macro [Merged by Bors] - Add @addlogprob! macro Jun 29, 2020
@bors bors bot closed this Jun 29, 2020
@bors bors bot deleted the addlogprob! branch June 29, 2020 12:29
@devmotion devmotion mentioned this pull request Aug 17, 2020
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.

3 participants