Zygote support - workaround setproperty! and getproperty

[mohamed82008]

This PR makes DynamicPPL compatible with Zygote by avoiding custom setproperty! and getproperty for vi::VarInfo. Zygote errors when using the custom definitions that we had.

[devmotion]

Maybe one could use getlogp and acclogp! instead of vi.logp[] and vi.logp[] += ... in most/all places, since they are defined anyway, seem slightly more readable, and definitely more general?

[mohamed82008]

Sounds good.

[devmotion]

I guess the same applies to TuringLang/Turing.jl#783 🙂

[devmotion]

I still think it's good to use getlogp, setlogp!, and acclogp, but maybe the issue (at least for getproperty) could be fixed by defining a custom adjoint for literal_getproperty, as suggested in the documentation of ZygoteRules.

[devmotion]

It seems the tests are copied from Turing's implementation, so my comments/questions here are the same as in the Turing PR.

[devmotion]

Just a general question: It seems some files in test/* copied from Turing. Are they modified in any way? Otherwise, wouldn't it be possible to run downstream tests by calling/executing the corresponding test files in Turing and just add Turing as a test dependency?

[mohamed82008]

Just a general question: It seems some files in test/* copied from Turing. Are they modified in any way? Otherwise, wouldn't it be possible to run downstream tests by calling/executing the corresponding test files in Turing and just add Turing as a test dependency?

It's less than ideal but see #18 for more details.

[codecov]

Codecov Report

Merging #31 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
- Coverage   88.88%   88.85%   -0.04%     
==========================================
  Files           9        9              
  Lines         729      727       -2     
==========================================
- Hits          648      646       -2     
  Misses         81       81

Impacted Files	Coverage Δ
src/DynamicPPL.jl	100% <ø> (ø)	⬆️
src/prob_macro.jl	93.75% <100%> (ø)	⬆️
src/varinfo.jl	87.95% <100%> (+0.22%)	⬆️
src/compiler.jl	93.85% <100%> (ø)	⬆️
src/selector.jl	80% <0%> (-20%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 177017f...1b6e5fc. Read the comment docs.

[devmotion]

Yeah, we experienced similar problems in JuliaDiffEq with, e.g., breaking changes in DiffEqBase that affected OrdinaryDiffEq or changes in OrdinaryDiffEq that broke DelayDiffEq.

[mohamed82008]

@devmotion any objections to merging this once tests pass?

[devmotion]

No, LGTM.