Closed Raphael-Tresor closed 2 months ago
That is indeed true @Raphael-Tresor ! Do you want to make a PR? I would suggest to do both, be a bit more explicit in the documentation and implement a new name and deprecate (but not remove) the old one.
@bvdmitri yes I would like to, I am assigning the issue to myself. Ok, I will do that.
merge request for ReactiveMP : https://github.com/ReactiveBayes/ReactiveMP.jl/pull/381
It’s not completed, it has been fixed only in ReactiveMP, not RxInfer. Reopening this
merge request for Graph PPL: https://github.com/ReactiveBayes/GraphPPL.jl/pull/172
Subject
Type FullFactorisation
struct FullFactorisation end
Issue
The name is confusing, people are likely to think that the underlying function will be constraint to be fully factorise even if it means the opposite:
$$q(x_1, x_2, x_3) = q(x_1) q(x_2) q(x_3) $$
$$ q(x_1, x_2, x_3) = q(x_1, x_2, x_3) $$
Part of the code concern
ReactiveMP
https://github.com/ReactiveBayes/ReactiveMP.jl/blob/cb809740496e92ab44d9312aa778d3c386e3b42d/src/node.jl#L139
RxInfer documentation
https://github.com/ReactiveBayes/RxInfer.jl/blob/495baa7183e7fa35e19c1e4dc3d44c982f31dcb1/docs/src/manuals/model-specification.md?plain=1#L335C1-L354C1
1.
MeanField()`Automatically specifies a mean-field factorisation
Example:
FullFactorisation()
Automatically specifies a full factorisation
Example:
`
Proposals to fix it
Only make it explicit in the documentation
OR
Change to an unambiguous name and make it explicit in the documentation