Export LogDensityFunction

[penelopeysm]

Closes #816

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.60%. Comparing base (4bc0d1f) to head (b699b7b).
Report is 1 commits behind head on release-0.35.

Additional details and impacted files

@@              Coverage Diff              @@
##           release-0.35     #820   +/-   ##
=============================================
  Coverage         84.60%   84.60%           
=============================================
  Files                34       34           
  Lines              3832     3832           
=============================================
  Hits               3242     3242           
  Misses              590      590           

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

[coveralls]

Pull Request Test Coverage Report for Build 13530876693

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 3 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-11.4%) to 73.244%

---

Files with Coverage Reduction	New Missed Lines	%
src/varinfo.jl	3	82.82%

Totals
Change from base Build 13530836280:	-11.4%
Covered Lines:	2795
Relevant Lines:	3816

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 13530876693

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 3 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-11.4%) to 73.244%

---

Files with Coverage Reduction	New Missed Lines	%
src/varinfo.jl	3	82.82%

Totals
Change from base Build 13530836280:	-11.4%
Covered Lines:	2795
Relevant Lines:	3816

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 13530876693

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 3 unchanged lines in 1 file lost coverage.
• Overall coverage remained the same at 84.692%

---

Files with Coverage Reduction	New Missed Lines	%
src/varinfo.jl	3	82.82%

Totals
Change from base Build 13530836280:	0.0%
Covered Lines:	3242
Relevant Lines:	3828

---

💛 - Coveralls

[mhauru]

Happy with this, unless we are going to what @yebai proposed and merge DynamicPPL.LogDensityFunction and AbstractMCMC.LogDensityModel, in which case we might be exposing an interface we are just about to change.

[penelopeysm]

They are different: DynamicPPL.LogDensityFunction is something that obeys the LogDensityProblems interface, whereas AbstractMCMC.LogDensityModel is something that wraps something that obeys the LogDensityProblems interface. (Mouthful, I know.)

I think that this in turn is because the LDP interface doesn't use abstract types. Thus, if you have a type T that obeys the LDP interface, you can't do

struct T <: ObeysLDPInterface
    fields...
end

sample(::ObeysLDPInterface,
    ::AbstractSampler,
    ...
)

Instead, if you want to sample from something that obeys it, you have to wrap it in a LogDensityModel and then you can dispatch on it.

I do find all this indirection a bit confusing and I would love to be able to simplify it, but I'm not sure if it's possible, and in any case I don't think it's a matter for this release

[yebai]

Can we make AbstractMCMC.LogDensityModel obey the LDP interface? This might be breaking, but we could still do it if it simplifies the code.

EDIT: if AbstractMCMC.LogDensityModel obeys the LDP interface, we can use it to replace DynamicPPL.LogDensityFunction.

[penelopeysm]

@yebai How about making LogDensityFunction subtype AbstractModel? I think that might sort it out. But it needs slightly deeper investigation, and I think it's best we leave this for another minor version.

As I understand it, the reason why LogDensityModel doesn't obey LDP is mainly to avoid method ambiguities: TuringLang/AbstractMCMC.jl#110 (comment)

[mhauru]

Merging the two sounds great if it works out. However, on a call Penny also just pointed out that Turing.jl already exports LogDensityFunction. In that case I think it should be exported in DPPL as well, and doing so doesn't cause any more breakage in the future, because any breakage of LDF is breaking for Turing.jl already as is. Hence happy to merge this and release it in v0.35 even if the LDF-LDM merging is going to happen soonish.

[penelopeysm]

I'll merge first, and I'll open another issue to track the possibilities.