Update for DynamicPPL 0.33 and 0.34

[penelopeysm]

This PR:

• removes the implementation of predict, which has been moved to DynamicPPL (Move predict from Turing DynamicPPL.jl#716)
• modifies the signature of values_as_in_model as needed

DPPL 0.34 removed certain methods of values_as_in_model, but those weren't used by Turing, so this release will be compatible with both 0.33 and 0.34.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.45%. Comparing base (24d5556) to head (d91cb48).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2459      +/-   ##
==========================================
- Coverage   85.01%   84.45%   -0.57%     
==========================================
  Files          21       21              
  Lines        1582     1570      -12     
==========================================
- Hits         1345     1326      -19     
- Misses        237      244       +7     

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

[coveralls]

Pull Request Test Coverage Report for Build 12877203451

Details

• 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• 8 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+8.0%) to 84.459%

---

Files with Coverage Reduction	New Missed Lines	%
src/mcmc/Inference.jl	8	80.89%

Totals
Change from base Build 12786420948:	8.0%
Covered Lines:	1326
Relevant Lines:	1570

---

💛 - Coveralls

[penelopeysm]

@sunxd3 Pinging you since the main breaking change (apart from me messing with values_as_in_model) was predict. Is it ok to just remove the function here entirely? :)

[sunxd3]

Is it ok to just remove the function here entirely?

I think so. The tests for predict are here. Not certain we want to keep a subset of them just to test the interface (probably not necessary).

[sunxd3]

Another question: currently Turing's predict is imported from StatsBase, while DPPL's predict is from AbstractPPL. Because AbstractPPL is implementing StatsBase.predict, so Turing's predict should be same as PDDL's. (Sorry for the tongue twister.)

But should be start using AbstractPPL's predict?

[penelopeysm]

tests

Oh, good catch - the tests should definitely be only in DynamicPPL, I don't think there's a need to keep them here. Will remove

imports

That's a good point! I was wondering the same about sample, actually, which is from AbstractMCMC but that's in turn also from StatsBase (I think). I'll investigate this properly on Monday if I get the time:)

[penelopeysm]

I looked into it a bit and I feel like the most sensible option is this:

• in DynamicPPL, overload and export AbstractPPL.predict
• in Turing, import and re-export DynamicPPL.predict (I think it all resolves to the same function? after all, DPPL doesn't have an issue importing AbstractPPL.predict when it's actually from StatsBase). The point being that we could import StatsBase.predict in Turing, but I feel like that kind of hides the fact that we're substantially overloading its behaviour in DynamicPPL, and importing predict from DynamicPPL at least reminds us where we're getting the implementation from.

Since we fixed the immediate issue with :=, would you feel comfortable re-exporting predict from DPPL?

[sunxd3]

On the importing/exporting front, I don't have more to add to this PR.

But maybe delete the tests for predict?

[penelopeysm]

Yup, I got rid of the tests already!

Re the exports I just realised AbstractPPL doesn't export predict either, so that's why we probably have to import it from StatsBase. let's keep it that way for now (until we sort out the exports in AbstractPPL/DynamicPPL 😬)