search

tweag / monad-bayes

A library for probabilistic programming in Haskell.
MIT License
410 stars 62 forks source link

Why do we have two functions called `mh` #194

Closed idontgetoutmuch closed 2 years ago

idontgetoutmuch commented 2 years ago

Describe the bug

Why do we have two functions called mh

https://github.com/tweag/monad-bayes/blob/notebooks/src/Control/Monad/Bayes/Traced/Static.hs#L87 https://github.com/tweag/monad-bayes/blob/notebooks/src/Control/Monad/Bayes/Inference/Lazy/MH.hs#L20

reubenharry commented 2 years ago

Lazy vs Strict. Different modules. If you prefer, we can call the second mhLazy, but I prefer not proliferating names without good reason.

reubenharry commented 2 years ago

You'll notice we actually have 4 mh functions in total, all in separate modules.

idontgetoutmuch commented 2 years ago

I suppose it was one of them having a different type that confused me

mh :: forall a. Double -> Weighted Sampler a -> IO [(a, Log Double)]
https://github.com/tweag/monad-bayes/blob/notebooks/src/Control/Monad/Bayes/Inference/Lazy/MH.hs#L20

mh :: MonadSample m => Int -> Traced m a -> m [a]
https://github.com/tweag/monad-bayes/blob/d8d3beb06cbaa09dc87922d76b6914a8c4e756c0/src/Control/Monad/Bayes/Traced/Basic.hs#L82

mh :: MonadSample m => Int -> Traced m a -> m [a]
https://github.com/tweag/monad-bayes/blob/6aa638660b942404dfc6c01c687d64445394142e/src/Control/Monad/Bayes/Traced/Static.hs#L87

mh :: MonadSample m => Int -> Traced m a -> m [a]
https://github.com/tweag/monad-bayes/blob/d8d3beb06cbaa09dc87922d76b6914a8c4e756c0/src/Control/Monad/Bayes/Traced/Dynamic.hs#L102
reubenharry commented 2 years ago

Yes, that's a fair point. The lazy one is sort of a different algorithm, so I now agree it could be renamed. Perhaps to mcmc or lazyMCMC.

turion commented 2 years ago

All those having the same type could be joined in a type class, possibly.

reubenharry commented 2 years ago

I thought that, but my attempt to do so led me to think that type families would have to be involved. Would be happy to be proven wrong.

turion commented 2 years ago

type families shouldn't be necessary. But there is some design space as to whether you want the type class to apply to each Traced monad transformer or to the Traced m monad. I think it's more usual to do the latter, but then one can't write down the current type signature of mh. (mhStep is fine though.)