Implement of

[sunxd3]

address #324

[github-actions]

JuliaBUGS.jl documentation for PR #331 is available at:
https://TuringLang.github.io/JuliaBUGS.jl/previews/PR331/

[yebai]

I took a quick look and left a few minor comments.

[yebai]

One issue with such tuple syntax is variables x, y, etc won't be available in the function body since Julia sees the first argument (x, y, ...) as a single argument by default. We can override this behaviour, but that might surprise users.

[github-actions]

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

JuliaFormatter

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 130 in 9b57a62

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 137 in 9b57a62

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 141 in 9b57a62

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 148 in 9b57a62

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 158 in 9b57a62

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 167 in 9b57a62

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Lines 169 to 176 in 9b57a62

	spec3 = of((
	a = of(Real),
	b = of((
	x = of(Array, 2),
	y = of(Real, 0, nothing)
	))
	))
	val3 = (a = 1.0, b = (x = [2.0, 3.0], y = 4.0))

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 185 in 9b57a62

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Lines 187 to 188 in 9b57a62

	spec = of((a = of(Array, 2), b = of(Real)))

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 191 in 9b57a62

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 194 in 9b57a62

---

[JuliaFormatter] _{reported by reviewdog 🐶}

JuliaBUGS.jl/test/test_of_type.jl

Line 200 in 9b57a62

end

[sunxd3]

@yebai I just pushed some local changes. Can you give it another quick read?

https://github.com/TuringLang/JuliaBUGS.jl/blob/sunxd/of/of_design_doc.md

('unflatten' doesn't quite work yet, but the idea should be there.)

[yebai]

A few comments:

1. Support dimention(of_type) to retrieve the dimension of of types or instances
2. Replacevalidated_params = validate(Tparams, user_input) with Tparams(user_input), i.e. use a constructor instead of validate API. The namedtuple-based constructor is more intuitive.
3. I don't see the motivation of the @of macro. All we need is already captured by of(), which has all the type information for defining a new type if required.
4. All LHS variables of tilde should be from the destructuring of the first input argument.

# always do 
@model function school_model((; mu0, beta, tau2, sigma2, school_effects)::SchoolParams, data)
end

# instead of 
@model function school_model(params::SchoolParams, data)
    (; mu0, beta, tau2, sigma2, school_effects) = params
end

The motivation is that we want to enforce that all LHS of tilde has to be a variable from the destructured first argument of models (e.g. mu0, beta, etc in school_model((; mu0, beta, tau2, sigma2, school_effects)::SchoolParams, ...) later. This will help us resolve the long-standing issue. In the above new syntax, a variable from the first argument doesn't tell whether it is a parameter or data. Instead, the "observation" step will be decoupled, and performed using the condition/decondition functions. This is slightly different from the current behaviour in DynamicPPL.model that input arguments on tilde's LHS will always be data, which will be deprecated.

5. Support for constants to delay specifying size information

Tp = of((;n=of(Constant), m=of(Constant), x=of(Array,n,m)))
zero(Tp, n=4, m=3)
rand(Tp, n=4, m=3)
Tp(n=4, m=3) # equivalent to `zero(Tp, n=4, m=3)`

[sunxd3]

there are some issues

• to support something like x::of(Real, ...) of need to return a Type object. the most straightforward thing to do is to return Real (as in of(Real) == Real) the issue is that Real doesn't carry extra information like lower and upper bounds
• I added the macro @of to be able to use it on the RHS of ::

[sunxd3]

@yebai comments at #331 (comment) are addressed.

couple of deviations:

1. use size and length instead of dimension, we can bind dimension to length, of course
2. Tp = of((;n=of(Constant), m=of(Constant), x=of(Array,n,m))) is hard to make work, I opted for Tp = of((;n=of(Constant), m=of(Constant), x=of(Array,:n,:m)))

The updated design doc is at https://github.com/TuringLang/JuliaBUGS.jl/blob/sunxd/of/of_design_doc.md and code is at https://github.com/TuringLang/JuliaBUGS.jl/blob/sunxd/of/src/of_type.jl. The source code required no dependencies, all the example code in the design doc should run, except for the model macro (I haven't made substantial updates to JuliaBUGS.@modelyet to reflectof`.

[yebai]

You can handle variable bounds using of(Constant), too, similar to the shape information of arrays.

Tp = of((;n=of(Constant), m=of(Constant), x=of(Array,n,m))) is hard to make work, I opted for Tp = of((;n=of(Constant), m=of(Constant), x=of(Array,:n,:m)))

This could be a good motivation for the @of macro. We could use of() for Real, Array etc, but always use @of for composable types including NamedTuples. Then we can do Tp = @of(n=of(Constant), m=of(Constant), x=of(Array,n,m))

Also, I don't think we need julia_type. It is never used in the example. If we do need it later, we can always do something like typeof(zero(of(Real))).

[sunxd3]

#324 (comment) is mostly implemented. Updated design doc https://github.com/TuringLang/JuliaBUGS.jl/blob/sunxd/of/of_design_doc.md.

The part left over is support of expressions in array size. I.e., currently

Tparams = @of(
   b=of(Int), 
   θ=of(Array, b, 1), 
   y=of(Real)
)

is fine, but

Tparams = @of(
   b=of(Int), 
   θ=of(Array, b+1, 1), 
   y=of(Real)
)

is not.

Two things to consider further:

• Julia constraints what can be used as type parameter

julia> A{:(x+1)}
ERROR: TypeError: in Type, in parameter, expected Type, got a value of type Expr
Stacktrace:
 [1] top-level scope
   @ REPL[2]:1

julia> A{:x}
A{:x} 

there are probably ways to circumvent this, but not trivial.

• what functions to allow in these size expressions

[yebai]

Good work @sunxd3 -- I left a few more comments above. We are getting there.

what functions to allow in these size expressions

For now, let's only support *, +, -, and /.

[sunxd3]

design at the moment: https://github.com/TuringLang/JuliaBUGS.jl/blob/sunxd/of/of_design_doc.md

use of expressions is also supported (an example is added at https://github.com/TuringLang/JuliaBUGS.jl/blob/sunxd/of/of_design_doc.md#bayesian-nonparametric-clustering-model)

[yebai]

Good work -- I added a few final comments on constructor syntax, mainly for consistency.

[yebai]

Suggested change

	rand(ExpandedMatrixType; n=10)
	rand(of(ExpandedMatrixType; n=10))

[yebai]

@sunxd3 For clarity, let's make sure to remove these methods

zero(ExpandedMatrixType; n=10)
rand(ExpandedMatrixType; n=10)
flat = flatten(MatrixType, instance; rows=3, cols=4)
unflatten(MatrixType, flat; rows=3, cols=4)

in favour of

zero(of(ExpandedMatrixType; n=10))
rand(of(ExpandedMatrixType; n=10))
flat = flatten(of(MatrixType; rows=3, cols=4), instance)
unflatten(of(MatrixType; rows=3, cols=4), flat)

for the principle that there should be only one way of doing things.

[sunxd3]

done now

[sunxd3]

the design doc is updated

[yebai]

Excellent -- we now have a complete of-type design!