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

[Merged by Bors] - Define rand defaults for AbstractProbabilisticProgram #79

Closed
wants to merge 5 commits into from

Conversation

sethaxen
Copy link
Member

This PR adds a 3-arg form of rand (suggested by @devmotion in TuringLang/DynamicPPL.jl#466 (comment)) to the interface for AbstractProbabilisticProgram and implements the default 1- and 2-arg methods that dispatch to this.

Currently tests fail because this breaks the fallbacks for GraphPPL.Model, which expects rand to forward to its rand! method. I'm not certain how we want to define the interface for this Model.

src/abstractprobprog.jl Show resolved Hide resolved
src/abstractprobprog.jl Outdated Show resolved Hide resolved
Co-authored-by: David Widmann <[email protected]>
@sethaxen
Copy link
Member Author

This code should also be backported to v0.5

@sunxd3
Copy link
Member

sunxd3 commented Feb 25, 2023

For now, I think we can just implement the new rand function by calling the existing rand.

@sunxd3
Copy link
Member

sunxd3 commented Feb 25, 2023

bors r+

@bors
Copy link

bors bot commented Feb 25, 2023

👎 Rejected by too few approved reviews

@sunxd3
Copy link
Member

sunxd3 commented Feb 25, 2023

bors r+

bors bot pushed a commit that referenced this pull request Feb 25, 2023
This PR adds a 3-arg form of `rand` (suggested by @devmotion in TuringLang/DynamicPPL.jl#466 (comment)) to the interface for `AbstractProbabilisticProgram` and implements the default 1- and 2-arg methods that dispatch to this.

Currently tests fail because this breaks the fallbacks for `GraphPPL.Model`, which expects `rand` to forward to its `rand!` method. I'm not certain how we want to define the interface for this `Model`.

Co-authored-by: Xianda Sun <[email protected]>
@bors
Copy link

bors bot commented Feb 25, 2023

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Define rand defaults for AbstractProbabilisticProgram [Merged by Bors] - Define rand defaults for AbstractProbabilisticProgram Feb 25, 2023
@bors bors bot closed this Feb 25, 2023
@bors bors bot deleted the rand branch February 25, 2023 06:09
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