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

Use of arbitrary distributions from Distributions.jl #79

Closed
bartvanerp opened this issue Jun 12, 2023 · 6 comments · Fixed by #82
Closed

Use of arbitrary distributions from Distributions.jl #79

bartvanerp opened this issue Jun 12, 2023 · 6 comments · Fixed by #82
Assignees
Labels
next version Features considered for the next version of the library

Comments

@bartvanerp
Copy link
Member

As discussed, it should be possible for users to use arbitrary distributions from Distributions.jl, without them needing to be defined in RxInfer.jl. Here just a reminder: As shown, this will require some lines of code, once the new GraphPPL version gets integrated in RxInfer. Would be great if the interface names get automatically adopted from the fieldnames in Distributions.jl.

@bartvanerp bartvanerp added the next version Features considered for the next version of the library label Jun 12, 2023
@wouterwln
Copy link
Member

wouterwln commented Jun 12, 2023

Note to self:

GraphPPL.NodeBehaviour(::Type{<:Distributions.Distribution}) = GraphPPL.Stochastic()
GraphPPL.rhs_to_named_tuple(::GraphPPL.Atomic, t::Type{<:Distributions.Distribution}, interface_values) = NamedTuple{fieldnames(t)}(interface_values)
GraphPPL.interfaces(t::Type{<:Distributions.Distribution}, ::Val{3}) = (:out, fieldnames(t)...)

These lines implement whatever is necessary in order to create arbitrary nodes from Distributions.jl objects. Preferably we put this in RxInfer.jl since I don't want to take a dependency on Distributions.jl in GraphPPL.jl. This also gives some weird error if yo do Normal(0, 1, 1) or any other illegal stuff which we should handle appropriately but at least it provides a MWE in order to make arbitrary distributions possible with the desired interface names.

@bvdmitri
Copy link
Member

We definitely put this either in RxInfer or in an extension (if Distributions is loaded by a user, then we could define extra functions, this is new in 1.9)

@wouterwln wouterwln linked a pull request Jun 26, 2023 that will close this issue
@wouterwln
Copy link
Member

I merged a PR that should implement the extension. @bvdmitri are you aware of any way in which I can test this or should we leave it be?

@wouterwln
Copy link
Member

wouterwln commented Jun 26, 2023

There are slight issues with this, as not all distributions from Distributions.jl have their interfaces as their struct fields (for example, a Dirichlet distribution has the following interfaces:

julia> GraphPPL.interfaces(Dirichlet, 1)
(:out, :alpha, :alpha0, :lmnB)

We can implement these "special" cases in the extension as well, but that kind of defies the point of this extension. Any suggestions?

@bvdmitri
Copy link
Member

@wouterwln I think you can load Distributions in the tests and try to create an arbitrary graph with some arbitrary distribution and test that interfaces match your expectations.

With the second issue about differences in the struct fields and desired interfaces it will happen more often I believe. You should look into the params method, but I'm not sure how to get actual names. That might require some manual work indeed.

@wouterwln
Copy link
Member

This is as fixed as it's going to be. I'm going to close this to clean up the overview of open issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next version Features considered for the next version of the library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants