bearriver: definitions missing

[ivanperez-keera]

BearRiver is missing some definitions available in Yampa. Ideally, and this is a big issue, it should include everything.

One should not worry at all about optimisations. Leaks and similar issues should be monitored and addressed systematically later.

[thalerjonathan]

I am currently experimenting with refactoring my code-base to Bearriver and need primitives like 'after', 'occationally' and 'repeatedly'. I will implement them and can then add them into Bearriver (and/or Dunai) if required.

[turion]

I can recommend using exceptions for that. E.g. repeatedly should go something like this:

import Control.Monad.Trans.MSF.Except

repeatedly diff b = safely $ do
  _ <- try $ proc _ -> do 
     time <- integrate    -< 1
     _      <- throwOn () -< time > diff
     step $ return b
     safe $ repeatedly diff b

Possibly there is an issue commuting ReaderT past ExceptT, see this for inspiration.

[thalerjonathan]

I just realised that 'after' is implemented already.

Regarding 'occasionally': Yampa gets its stream of random-numbers from a RandomGen which is passed as an argument to occasionally. With MSFs it would be possible to have a RandT monad somewhere in the stack from which to draw the random-numbers. I guess fixing it in the types would be too restrictive / wouldnt work as we fix the monad stack. A solution would be to have a function which generates a random-number from the existing monad stack - is this possible?

[turion]

A clean solution to this would maybe be the following API:

module Control.Monad.Trans.MSF.Rand where

-- MonadRandom
import Control.Monad.Random

-- | Updates the generator every step
runRandS :: MSF (RandT g m) a b -> g -> MSF m a (g, b)

evalRandS  :: MSF (RandT g m) a b -> g -> MSF m a b
evalRandS msf g = runRandS msf g >>> arr snd

[turion]

Getting a stream of random numbers is then as easy as calling arrM_ getRandom. There should probably be an alias for it then, called getRandomS :: (Random b, MonadRandom m) => MSF m a b.

[thalerjonathan]

Here is my naive implementation of occasionally in Yampa style (passing in RandomGen explicitly):

occasionally :: (RandomGen g, Monad m) => g -> Time -> b -> SF m a (Event b)
occasionally g t_avg b
    | t_avg > 0 = MSF (const $ tf g b)
    | otherwise = error "AFRP: occasionally: Non-positive average interval."
  where
    tf :: (RandomGen g, Monad m) => g -> b -> ClockInfo m (Event b, SF m a (Event b))
    tf g b = do
      dt <- ask
      let (r, g') = randomR (0, 1) g
      let p = 1 - exp (-(dt / t_avg))
      let evt = if r < p 
                  then Event b 
                  else NoEvent
      return (evt, MSF (const $ tf g' b))

[turion]

Why is there no instance (MonadTrans t, MonadRandom m) => MonadRandom (t m) in the MonadRandom package? This would be really useful for us.

[turion]

Something like this might work. It would be nicer though to first write a more general purpose thing like getRandomS and then implement occasionally in terms of it. It's discouraged to use the MSF constructor, and you don't need it here (feedback works as well).

[thalerjonathan]

What about this implementation:

occasionallyFeedback :: (RandomGen g, Monad m) => g -> Time -> b -> SF m a (Event b)
occasionallyFeedback g t_avg b
    | t_avg > 0 = proc _ -> do
      r <- getRandomS g -< ()
      let p = 1 - exp (-(dt / t_avg))
      if r < p
        then returnA -< Event b
        else returnA -< NoEvent
    | otherwise = error "AFRP: occasionally: Non-positive average interval."

getRandomS :: (RandomGen g, Random b, Monad m) => g -> SF m a b
getRandomS g0 = feedback g0 getRandomSAux
  where
    getRandomSAux = proc (_, g) -> do
      let (r, g') = random g
      returnA -< (r, g')

[turion]

Looking good :) do you think you can implement runRandS as well? (Or is it unclear how transformers and MSFs interact?) RandT is basically StateT, so it should be possible to reuse the code from Control.Monad.Trans.MSF.State.

[thalerjonathan]

It's not yet clear (I am just beginning to understand Monad Transformers). I want to completely omit the RandomGen argument in occasionally and instead draw from a RandT monad which is somewhere in the monad-transformer stack. I don't know how to set up the correct types so that it has a general solution and works for all monad transformers.

Lets say one has the following:

occasionallyMSF :: RandomGen g => Time -> b -> SF (StateT (AgentOut s m) (Rand g)) a (Event b)

The question is now how to formulate this into a general solution which works for the above SF but also for other SFs which have a RandT somewhere in the stack? I thought about this approach but then this is not compatible with the above SF as StateT comes before Rand (I didn't fully understand the order of transformers):

occasionallyMSFGeneral :: (Monad m, RandomGen g) => Time -> b -> SF (RandT g m) a (Event b)

[turion]

For most transformers, there are two aspects: The transformer itself, i.e. RandT, and the corresponding class, i.e. MonadRandom. The class shouldn't care about where RandT is sitting in the stack. (That's what I meant by "there should be an instance (MonadTrans t, MonadRandom m) => MonadRandom (t m)". It says that you can bury a RandT under arbitrary other transformers and still access its functionality.) This means we'd have to implement the following for a completely clean solution:

-- Yep, an orphan instance, sadly. Eventually this should be a pull request to the MonadRandom package.
instance (MonadTrans t, MonadRandom m) => MonadRandom (t m) where
  -- use 'lift' from transformers here

-- | Updates the generator every step
runRandS :: MSF (RandT g m) a b -> g -> MSF m a (g, b)
runRandS = _ -- Hint: Use the isomorphism 'RandT ~ StateT' and then 'Control.Monad.Trans.MSF.State'

evalRandS  :: MSF (RandT g m) a b -> g -> MSF m a b
evalRandS msf g = runRandS msf g >>> arr snd


occasionally :: MonadRandom m => Time -> b -> SF m a (Event b)
occasionally = _ -- You already have this basically

[thalerjonathan]

Actually my Feedback implementation was wrong as it used a global variable dt. Here is the correct version:

occasionallyFeedback :: (RandomGen g, Monad m) => g -> Time -> b -> SF m a (Event b)
occasionallyFeedback g t_avg b
    | t_avg > 0 = proc _ -> do
      r <- getRandomS g -< ()
      dt <- timeDelta -< ()
      let p = 1 - exp (-(dt / t_avg))
      if r < p
        then returnA -< Event b
        else returnA -< NoEvent
    | otherwise = error "AFRP: occasionally: Non-positive average interval."

timeDelta :: Monad m => SF m a DTime
timeDelta = arrM_ ask

[thalerjonathan]

Here is the implementation without an explicit RandomGen:

occasionally :: MonadRandom m => Time -> b -> SF m a (Event b)
occasionally t_avg b
  | t_avg > 0 = proc _ -> do
    r <- getRandomRS (0, 1) -< ()
    dt <- timeDelta -< ()
    let p = 1 - exp (-(dt / t_avg))
    if r < p
      then returnA -< Event b
      else returnA -< NoEvent
  | otherwise = error "AFRP: occasionally: Non-positive average interval."

getRandomRS :: (MonadRandom m, Random b) => (b, b) -> SF m a b
getRandomRS r = proc _ -> do
  r <- arrM_ $ getRandomR r -< ()
  returnA -< r 

[thalerjonathan]

And here is the implementation for runRandS (follwing your State implementation):

runRandS :: (RandomGen g, Monad m) => MSF (RandT g m) a b -> g -> MSF m a (g, b)
runRandS msf g = MSF $ \a -> do
  ((b, msf'), g') <- runRandT (unMSF msf a) g
  return ((g', b), runRandS msf' g')

[turion]

Looking good! Definitely make a pull request out of this!

Just a few minor stylistic things:

• We shouldn't say "AFRP" in error messages, but rather just "dunai".
• I think we do camelCase rather than underscores as in t_avg.
• -< should be aligned.
• It's probably more efficient to have the if inside returnA value, i.e.:

returnA -< if r < p then Event b else NoEvent

Like this, it's just a function. Your code is correct nevertheless, but it uses ArrowChoice, which could be slightly slower.

[thalerjonathan]

OK, I will start making the transition in my library from Yampa to Dunai / BearRiver tomorrow and will then come up with a few pull requests.

Thanks for the great help and hints, this really helped me to better understand MSFs and Monadic Transformers!

[thalerjonathan]

I created the pull request. I forgot to remove Werror before commiting and Travis failed to run through - removed it and all was OK. Why not compile with Werror?

[ivanperez-keera]

Thanks a lot @turion for following along and helping @thalerjonathan with his contribution :)

Thanks, @thalerjonathan for the commits.

I agree with you in principle that it would be a great idea to compile everything with Werror. Some warnings are unavoidable, like orphan instances (which we use). The only solution when using Werror is to instruct GHC, in the cabal file or with GHC option pragmas in the haskell files, to ignore certain kinds of warnings.

Minor suggestion added as a review. I may be wrong. In general, in haskell, whenever you find yourself writing:

a <- someArrow -< b
returnA -< a

or

do
  a <- someMonadicExpression
  return a

consider if you can just write the thing without the a. This is a general rule, but there are exceptions. Variable names help document things. Sometimes the expression (someMonadicExpression) does not clearly state what's going on at the level of abstraction that you may be using in the current file, and a well-named variable sometimes solves the problem.

I would suggest aligning things in general. I believe it tends to help understand code and detect anomalies (we humans are good at noticing what's different). Again, this is not a hard-rule: sometimes aligning things introduces so many spaces that it makes things much harder to read.

Also, trying to stick to a fixed number of spaces for indentation (always 2, or always 4). This is more of a guideline. I have not been able to stick to it 100%, especially because it looks weird in some places, and some Haskell constructs don't make this easy.

As @turion already knows, I try to impose these "rules", but I'm the first to break them :( It's usually by mistake.

I want this to be accepted straight from you without changes, so I've just been super-annoying and added these as comments to the pull request.

Also, I would suggest, in the pull-request commit message, to reference the issue in question:

As discussed (#40).

Alternatively, give a very short description.

Adds occasionally, <something> and <something> (refs #40).

This will create a link from the issue to the commit, and from the commit to the issue.

Thanks!

[thalerjonathan]

@ivanperez-keera thanks for your comments and your patience. I totally agree there MUST be a consistent code style in a project where multiple people are working together (or contributing) otherwise everything would fall into chaos. I try to adapt it as quickly as possible and am grateful for any comments on style and best practices (e.g. point-free style).

[ivanperez-keera]

Not at all! Thank you for contributing! Open issues when you find more things need to be added/improved as you use the library :)

[turion]

Do we have everything from Yampa now or is there still stuff missing? What do we want to have? Should we open separate issues?

[ivanperez-keera]

Yampa is immensely big. We do not need to have "everything", but only elementary constructs (those that cannot be expressed in terms of others).

I think that's the core, the most generic parallel switch and decoupled switch, and async stuff (there's one async combinator, apparently).

[turion]

Can you say more precisely? I don't know about asynchronous stuff in Yampa. Maybe we can draw up a short list of modules that we want to have. We can add FRP.Yampa.Random thanks to @thalerjonathan 's contribution as well.

[ivanperez-keera]

This has been a long issue. We are now very close to finishing: only three modules away: Simulation, Tasks and Switches.

[ivanperez-keera]

Closed in favor of the smaller issues that address each module individually. We are not close enough to finishing that we no longer need this larger issue to track things.