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

probvec is defined for Dirichlet #228

Open
wouterwln opened this issue Feb 4, 2025 · 1 comment
Open

probvec is defined for Dirichlet #228

wouterwln opened this issue Feb 4, 2025 · 1 comment

Comments

@wouterwln
Copy link
Member

I don't think we should define probvec for Dirichlet since it returns the pseudo-count parameters for the Dirichlet distribution. I would say that a sample from a Dirichlet distribution is a probvec, and the output of a call to probvec should always be a normalized discrete distribution. I am referencing this because ReactiveBayes/ReactiveMP.jl#424 this PR actually found a bug in the tests for the Transition node. In this line we test the rule for the Transition node when the posterior marginal on the out interface is Dirichlet. This is something that is not possible since the Transition node governs transitions between Categorical variables (which can have a Dirichlet prior, sure, but the incoming message or marginal on that edge should be a Categorical distribution or PointMass. The test does not fail because Dirichlet implements probvec. Which I think is something then we should not be doing.

What do you think @ismailsenoz @Nimrais @ThijsvdLaar @bvdmitri ?

@bvdmitri
Copy link
Member

bvdmitri commented Feb 4, 2025

I agree something is fishy here, I think we should just use params() from Distributions.jl where necessary? I think probvec is a remnant from ForneyLab's implementation

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

No branches or pull requests

2 participants