search

stripe / rainier

Bayesian inference in Scala.
https://rainier.fit
Apache License 2.0
433 stars 51 forks source link

Implement generator traversal using implicits #361

Closed metcalf closed 5 years ago

metcalf commented 5 years ago

This re-implements Generator.traverse based on a GeneratorTraversal implicit trait. This allows for defining traversals for custom classes as demonstrated in the test. I'm pretty sure you could use shapeless with this to implement traversal for any case class where each member has a ToGenerator.

Assuming this looks good, I'll implement the same thing for RandomVariable.

codecov-io commented 5 years ago

Codecov Report

Merging #361 into develop will increase coverage by 0.18%. The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop    #361      +/-   ##
==========================================
+ Coverage    46.32%   46.5%   +0.18%     
==========================================
  Files           84      84              
  Lines         3050    3051       +1     
  Branches       135     142       +7     
==========================================
+ Hits          1413    1419       +6     
+ Misses        1637    1632       -5
Impacted Files Coverage Δ
...in/scala/com/stripe/rainier/core/Categorical.scala 22.41% <0%> (ø) :arrow_up:
.../scala/com/stripe/rainier/scalacheck/package.scala 92.85% <100%> (ø) :arrow_up:
...main/scala/com/stripe/rainier/core/Generator.scala 91.04% <84.61%> (+3.16%) :arrow_up:
...ain/scala/com/stripe/rainier/compute/RealOps.scala 79.66% <0%> (+2.54%) :arrow_up:

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 5c91f4a...a2831a2. Read the comment docs.

avi-stripe commented 5 years ago

I'd love to get @non or @andyscott's take on this.

avi-stripe commented 5 years ago

What I'm not sure about here is whether it's actually desirable to have a generic Generator.traverse which can handle arbitrary data structures, or whether it's better to have a small number of specific variants like traverseMap.

andyscott commented 5 years ago

I would flip this around and use existing machinery in cats:


traverse
avibryant commented 5 years ago

Closing this since I'm pretty sure we don't think this is the right approach.