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

Confusion regarding .~ #722

Open
torfjelde opened this issue Nov 27, 2024 · 3 comments
Open

Confusion regarding .~ #722

torfjelde opened this issue Nov 27, 2024 · 3 comments

Comments

@torfjelde
Copy link
Member

Ref: TuringLang/Turing.jl#2390 (comment)

MWE:

julia> @model demo(x) = x .~ Normal()
demo (generic function with 2 methods)

julia> demo(missing)()
ERROR: MethodError: no method matching dot_assume(::Random.TaskLocalRNG, ::SampleFromPrior, ::Normal{…}, ::VarName{…}, ::Missing, ::UntypedVarInfo{…})
[...]

julia> demo([missing])()
ERROR: MethodError: no method matching loglikelihood(::Normal{Float64}, ::Vector{Union{Missing, Float64}})
[...]

Even if you use condition, you still run into a similar issue when specifying components, i.e.

julia> @model function demo()
           x = Vector{Float64}(undef, 2)
           x .~ Normal()
       end
demo (generic function with 4 methods)

julia> (demo() | (@varname(x[1]) => 1.0,))()  # not conditioned
2-element Vector{Float64}:
 1.6841936984140546
 0.3645020534551503

The reason why we do this is because of performance: we want .~ to be faster than just iterating over the entire broadcast expression and calling tilde_assume!! and tilde_observe!! separately. However, this is very, very confusing for users, and I think it's very understandable.

Sooo the question is how to fix this. A few immediate thoughts:

  1. Do we need .~? Does it just make things confusing?
  2. Should we just convert it into a loop over the broadcasted expression and drop the perf benefits?
@mhauru
Copy link
Member

mhauru commented Nov 27, 2024

How significant are the perf benefits? Could we benchmark this?

Either of the proposed two solutions would also simplify the tilde pipeline code a lot, improving maintainability. It's a source of a lot of corner cases and code complexity.

@mhauru
Copy link
Member

mhauru commented Nov 27, 2024

We could ask on Slack/Discourse whether our users are making heavy use of .~, to gauge how much effort we should put into keeping it around and performant.

@yebai
Copy link
Member

yebai commented Nov 29, 2024

  1. Do we need .~? Does it just make things confusing?
  2. Should we just convert it into a loop over the broadcasted expression and drop the perf benefits?

I lean towards option 2 or something similar if it simplifies the implementation. The tilde pipeline is becoming a bit too (deep and) complex to reason about.

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

3 participants