Improving code to generate a distribution.

Question

Hey,

I am new to Haskell and I wonder how/if I can make this code more efficient and tidy. It seems unnecessarily long and untidy.

My script generates a list of 10 averages of 10 coin flips.

import Data.List
import System.Random

type Rand a = StdGen -> Maybe (a,StdGen)

output = do
    gen <- newStdGen
    return $ distBernoulli 10 10 gen

distBernoulli :: Int -> Int -> StdGen -> [Double]
distBernoulli m n gen = [fromIntegral (sum x) / fromIntegral (length x) | x <- lst]
    where lst = splitList (randomList (n*m) gen) n

splitList :: [Int] -> Int -> [[Int]]
splitList [] n = []
splitList lst n = take n lst : splitList (drop n lst) n

randomList :: Int -> StdGen -> [Int]
randomList n = take n . unfoldr trialBernoulli

trialBernoulli :: Rand Int
trialBernoulli gen = Just ((2*x)-1,y)
                 where (x,y) = randomR (0,1) gen

Any help would be appreciated, thanks.

Answer 1

I'm not sure I understand your code or your question...

But it seems to me all you'd need to do is generate a list of random ones and zeroes, and then divide each of them by their length with a map and add them together with a foldl.

Something like:

makeList n lis = if n /= 0 then makeList (n-1) randomR(0,1) : lis else lis

And then make it apply a Map and Foldl or Foldr to it.

Answer 2

EDIT: I posted this too fast and didn't match behavior, it should be good now.

import Control.Monad.Random
import Control.Monad (liftM, replicateM)

KNOWLEDGE: If you like randoms then use MonadRandom - it rocks.

STYLE: Only importing symbols you use helps readability and sometimes maintainability.

output :: IO [Double]
output = liftM (map dist) getLists

Note: I've given output an explicit type, but know it doesn't have to be IO.

STYLE:

1) Its usually good to separate your IO from pure functions. Here I've divided out the getting of random lists from the calculation of distributions. In your case it was pure but you combined getting "random" lists via a generator with the distribution function; I would divide those parts up.

2) Read Do notation considered harmful. Consider using >>= instead of

output = do
   gen <- new
   return $ dist gen

you can do:

output = new >>= dist

Wow!

dist :: [Int] -> Double
dist lst = (fromIntegral (sum lst) / fromIntegral (length lst))

getLists :: MonadRandom m => Int -> Int -> m [[Int]]
getLists m n= replicateM m (getList n)

KNOWLEDGE In Control.Monad anything ending in an M is like the original but for monads. In this case, replicateM should be familiar if you used the Data.List replicate function.

getList :: MonadRandom m => Int -> m [Int]
getList m = liftM (map (subtract 1 . (*2)) . take m) (getRandomRs (0,1::Int))

STYLE: If I do something lots of times I like to have a single instance in its own function (getList) then the repetition in a separate function.

Answer 3

I'd tackle this problem in a slightly different way. First I'd define a function that would give me an infinite sampling of flips from a Bernoulli distribution with success probability p:

flips :: Double -> StdGen -> [Bool]
flips p = map (< p) . randoms

Then I'd write distBernoulli as follows:

distBernoulli :: Int -> Int -> StdGen -> [Double]
distBernoulli m n = take m . map avg . splitEvery n . map val . flips 0.5
  where
    val True = 1
    val False = -1
    avg = (/ fromIntegral n) . sum

I think this matches your definition of distBernoulli:

*Main> distBernoulli 10 10 $ mkStdGen 0
[-0.2,0.4,0.4,0.0,0.0,0.2,0.0,0.6,0.2,0.0]

(Note that I'm using splitEvery from the handy split package, so you'd have to install the package and add import Data.List.Split (splitEvery) to your imports.)

This approach is slightly more general, and I think a little neater, but really the main difference is just that I'm using randoms and splitEvery.

Answer 4

Using the above, I am now using this.

import Data.List
import System.Random

type Rand a = [a]

distBernoulli :: Int -> Int -> StdGen -> [Double]
distBernoulli m n gen = [fromIntegral (sum x) / fromIntegral (length x) | x <- lst]
    where lst = take m $ splitList (listBernoulli gen) n

listBernoulli :: StdGen -> Rand Int
listBernoulli = map (\x -> (x*2)-1) . randomRs (0,1)

splitList :: [Int] -> Int -> [[Int]]
splitList lst n = take n lst : splitList (drop n lst) n

Thanks for your help, and I welcome any further comments :)