'FullFactorisation' is a confusing name for a constraint that impose no factorisation.

[Raphael-Tresor]

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:

• What I intuitively understood :

$$q(x_1, x_2, x_3) = q(x_1) q(x_2) q(x_3) $$

• What it actually is :

$$ 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:

y ~ NormalMeanVariance(y_mean, y_var) where { q = MeanField() }

2. FullFactorisation()

Automatically specifies a full factorisation

Example:

y ~ NormalMeanVariance(y_mean, y_var) where { q = FullFactorisation() }

`

Proposals to fix it

Only make it explicit in the documentation

OR

Change to an unambiguous name and make it explicit in the documentation

[bvdmitri]

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.

[Raphael-Tresor]

@bvdmitri yes I would like to, I am assigning the issue to myself.
Ok, I will do that.

[Raphael-Tresor]

merge request for ReactiveMP : ReactiveBayes/ReactiveMP.jl#381

[bvdmitri]

It’s not completed, it has been fixed only in ReactiveMP, not RxInfer. Reopening this

[Raphael-Tresor]

merge request for Graph PPL: ReactiveBayes/GraphPPL.jl#172