Remove dot_tilde pipeline

[mhauru]

This PR reimplements .~ so that it is turned into a loop over ~ statements at macro time. It also restricts the RHS of .~ to be a univariate distribution. This breaks code where either an array of distributions or a multivariate distribution was used on the RHS. The former case can easily be worked around by rather using ~ product_distribution(...) (and performance is in many cases improved by it), the latter case is presumably rare and can be worked around by using a loop.

All the real changes are in compiler.jl and the tests. Everything else is just deleting code that is no longer needed after this change.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.43%. Comparing base (4b9665a) to head (30c241f).
Report is 1 commits behind head on release-0.35.

Additional details and impacted files

@@               Coverage Diff                @@
##           release-0.35     #804      +/-   ##
================================================
- Coverage         86.32%   84.43%   -1.90%     
================================================
  Files                35       34       -1     
  Lines              4162     3823     -339     
================================================
- Hits               3593     3228     -365     
- Misses              569      595      +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[coveralls]

Pull Request Test Coverage Report for Build 13377908373

Details

• 37 of 37 (100.0%) changed or added relevant lines in 2 files are covered.
• 57 unchanged lines in 11 files lost coverage.
• Overall coverage decreased (-1.9%) to 84.525%

---

Files with Coverage Reduction	New Missed Lines	%
src/context_implementations.jl	2	81.25%
src/debug_utils.jl	2	0.0%
src/sampler.jl	3	90.63%
src/threadsafe.jl	4	57.14%
src/utils.jl	4	73.36%
src/distribution_wrappers.jl	5	38.89%
src/varinfo.jl	5	82.17%
src/values_as_in_model.jl	6	66.67%
src/extract_priors.jl	8	71.43%
src/compiler.jl	9	85.96%

Totals
Change from base Build 13376471008:	-1.9%
Covered Lines:	3228
Relevant Lines:	3819

---

💛 - Coveralls

[mhauru]

Note that this line was making us always test one single model, even though we were ostensibly in a loop over models. Presumably this got committed by accident at some point.

[mhauru]

If you remove the line a bit above, that was fixing the model, it turns out that even on master many of the models fail with linked SimpleVarInfos. Some passed, I'm not sure what it was that made the difference. Presumably some quirk of SimpleVarInfo interacted badly with some types of models.

With the new implementation of .~ all of the test models fail with linked SimpleVarInfos. I assume its the same problem as before, but now manifesting itself in all test models, due to changes in the test models. Hence commenting out these three. I wouldn't really call this a regression in tests, in that these only ever passed for a few select models, whereas now at least we check the unlinked SimpleVarInfos for all models.

[mhauru]

One single test in pointwise_logdensities fails still, something about converting to Chains. I'll figure it out next week. If anyone wants to review the main changes in compiler.jl in the meanwhile, feel free.

[torfjelde]

Will leave approval to someone who is more hands on, but really like this:)

One thing: seems like this would warrant a bump of the minor version given it's breaking?

[mhauru]

Thanks @torfjelde!

One thing: seems like this would warrant a bump of the minor version given it's breaking?

The PR is proposed to be merged to the release-0.35 branch which includes a bunch of other breaking changes and a minor version bump.

[mhauru]

The remaining test failures are Mooncake issues. Not sure what to do about them (mark as broken?), but it does make this ready for review.

[mhauru]

@willtebbutt confirms that the Mooncake test failures are an instance of compintell/Mooncake.jl#319, which in turn is caused by a Julia compiler bug.

[penelopeysm]

I'm happy to approve on the code changes, but I'm hesitant to click on the button because I'm still very curious about the Mooncake errors. I managed to get a simple model with a literal on the LHS to error (see compintell/Mooncake.jl#484 for context) and I'm extremely confused by why it happens only with one model and not the others, and only in some cases but not.

I'd like to understand, from the DynamicPPL point of view, what it is in that one model that gives it problems.

[penelopeysm]

I'm also aware that could be a huge time sink, and could easily be one day of good work.

[willtebbutt]

FWIW, I would only say that it's a good use of time to get to the bottom of why one DynamicPPL model triggers this only if you're keen to practice your compiler-debugging skills. I'm hoping to have a patch sorted by the end of the day, so if you're happy to hold off on pressing the button for a few hours, it should go away anyway.

[penelopeysm]

Rewriting

@model function f() # errors!
    s ~ InverseGamma(2, 3)
    m ~ Normal(0, sqrt(s))
    [1.5, 2.0] .~ Normal(m, sqrt(s))
end

into

@model function f() # fine!
    s ~ InverseGamma(2, 3)
    m ~ Normal(0, sqrt(s))
    [1.5, 2.0][(1,)...] ~ Normal(m, sqrt(s))
    [1.5, 2.0][(2,)...] ~ Normal(m, sqrt(s))
end

avoids the Mooncake error. In fact, even

@model function f() # also fine!
    s ~ InverseGamma(2, 3)
    m ~ Normal(0, sqrt(s))
    x = [1.5, 2.0]
    for idx in Iterators.product(axes(x)...)
        x[idx...] ~ Normal(m, sqrt(s))
    end
end

is perfectly fine. I don't know why the current implementation of .~ is problematic, but I wonder if there is a workaround that will let us circumvent the Mooncake issue.

---

BTW, while digging into this, I found that there are other demo models (e.g. demo_assume_observe_literal, but basically anything with a literal on the LHS) which Mooncake errors on if we call

This was an error on my part, see compintell/Mooncake.jl#484 (comment) for context.

[penelopeysm]

I concede defeat

[penelopeysm]

Maybe mark the tests as broken, but that's about it

[mhauru]

Over in compintell/Mooncake.jl#319 @willtebbutt said he wanted to see if he could put in a quick workaround, so I'll wait to hear from him first, but I'm happy to mark as broken if need be.

[penelopeysm]

Oh, okay, we can wait then!

[willtebbutt]

Just to double check I'm interpreting CI results correctly: am I right in saying that compintell/Mooncake.jl#319 is only causing this PR problems on Julia 1.11, not 1.10?

(This would make sense, as the offending code only appears to appear in the compiler in 1.11.)

[mhauru]

Just to double check I'm interpreting CI results correctly: am I right in saying that compintell/Mooncake.jl#319 is only causing this PR problems on Julia 1.11, not 1.10?

Yup, so it seems.

[willtebbutt]

Fab. I'm just waiting for CI to pass in compintell/Mooncake.jl#487 , then I'll make a release which should fix the problem.

[willtebbutt]

v0.4.90 ought to solve the problem :)

[penelopeysm]

Sorry, I didn't mean to rescind my original approval with that change above. Happy to merge!