-
Notifications
You must be signed in to change notification settings - Fork 37
[Merged by Bors] - Introducing AbstractPPL dependency #214
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
Conversation
devmotion
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just needs a version bump. I would also prefer to merge #217 first to ensure that all Turing tests still pass.
devmotion
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll approve the PR if the version is bumped and the comments are addressed 🙂 Then we can merge and release the changes.
devmotion
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
|
bors r+ |
This has three very rudimentary consequences: - `VarName` and its helpers are moved over to AbstractPPL completely - The new abstract base type for models is `AbstractPPL.AbstractProbabilisticProgram <: AbstractMCMC.AbstractModel` - `AbstractVarInfo <: AbstractPPL.AbstractModelTrace` More abstractions (and hopefully concrete generalizations, too) are about to come.
This has three very rudimentary consequences:
VarNameand its helpers are moved over to AbstractPPL completelyAbstractPPL.AbstractProbabilisticProgram <: AbstractMCMC.AbstractModelAbstractVarInfo <: AbstractPPL.AbstractModelTraceMore abstractions (and hopefully concrete generalizations, too) are about to come.