turion / essence-of-live-coding

Universal Live Coding & Functional Reactive Programming Framework
https://www.manuelbaerenz.de/#computerscience
64 stars 6 forks source link

Use the Has type class instead of explicit monad transformer stacks #103

Open miguel-negrao opened 2 years ago

miguel-negrao commented 2 years ago

Would it make sense to always use the Has type class in all packages here instead of explicit monad transformer stacks ? It would be a bit of work converting all functions to use Has, and type signatures would be a bit more abstract, but on the positive side it would mostly eliminate the use of lifts in eolc-based code, right ?

turion commented 2 years ago

This would be a cool experiment to see whether has-transformers is useful in a larger real-world library. It would only replace multiple lifts with one liftH.

miguel-negrao commented 2 years ago

But if the value is qualified by a constraint, e.g. f :: Has t1 m => m a and it is used in a monadic context of type (Has t1 m, Has t2 m, Has t3 m) => m b does one needs to use lift ? I was assuming it wouldn't. If that is the case, and if most eolc functions would be qualified with a Has constraint instead of having an explicit monad stack, then in most cases one wouldn't use lift, right ?

miguel-negrao commented 2 years ago

Ok, gave it a try, and indeed the case I mentioned doesn't seem to need lift:

{-# LANGUAGE FlexibleContexts #-}

module Main where

import Control.Monad.Trans.Has.Reader
  ( HasReader,
    ReaderT (runReaderT),
    ask,
  )
import Control.Monad.Trans.Has.State
  ( HasState,
    StateT (runStateT),
    put,
  )
import Lib

main :: IO ()
main = handled >>= print

a :: (Monad m, HasState Int m) => m ()
a = put (3 :: Int)

b :: (Monad m, HasState Int m, HasReader Int m) => m Int
b = do
  put (4 :: Int)
  a
  ask

handled :: IO (Int, Int)
handled = b `runReaderT` (10 :: Int) `runStateT` 0
turion commented 2 years ago

Ah right, if we change the type signatures of several functions, it would get even easier. I'd be open for a PR to try that out.

miguel-negrao commented 2 years ago

I had a go at this. For exceptions:

throwC
  :: forall m e arbitrary . (Monad m, HasExcept e m)
  => Cell m e arbitrary
throwC = arrM (\e -> liftH $ ExceptT $ return $ Left e) -- can't avoid lambda, doesn't know the type of n

The trade-off of using Has is that some additional type annotations are necessary as the full type has to be known for the compiler to determine which instance of Has to use. In some cases we don't care about the exception type e, and it is not known as it is thrown away, but with "Has" it has to be known, I think:

throwWhen0
  :: (Monad m, HasExcept () m)
  => Cell m Double Double
throwWhen0 = proc pos ->
  if pos < 0
  then throwC  -< ()
  else returnA -< pos

sineChangeE = do
  try @() $ sine 6 >>> throwWhen0
  try @() $ (constM $ lift $ putStrLn "I changed!")
      >>> throwC
  safe $ sine 10

Here, the exception type has to be explicitly determined (I used type application), which is very inconvenient...

For HandlingStateT things are more complicated because HandlingStateT m is not really just a monad transformer, but a monad transformer which has type m which has to match the monad inside the transformer (type HandlingStateT m = StateT (HandlingState m) m). For this I created a type class similar to Has but which determines if a whole bottom part of a monad transformer stack is present in a monad stack.

class HasMonad (inner :: * -> *) (outer :: * -> *) where
  liftHM :: inner a -> outer a

instance Monad m => HasMonad m m where
  liftHM = id
  {-# INLINE liftHM #-}

instance {-# Overlappable #-} (Monad inner, Monad outer, MonadTrans t, HasMonad inner outer) => HasMonad inner (t outer) where
  liftHM = lift . liftHM
  {-# INLINE liftHM #-}

handling' :: (HasMonad (HandlingStateT n) m, Typeable h, Monad n) => 
  Handle n h -> Cell m arbitrary h
handling' = hoistCell liftHM . handling

The problem here is that the type n is usually unknown. Even activating AmbiguousTypes quickly one gets into a lot or problems. E.g.:

readEventsC :: 
  forall m n arbitrary .
  (MonadIO m, MonadIO n, HasExcept EOLCPortMidiError m, HasMonad (HandlingStateT n) m)
  => String -> Cell m arbitrary [PMEvent]
readEventsC name = proc _ -> do
  pmStreamE <- handling' @n @m $ portMidiInputStreamHandle name -< ()
  pmStream <- exceptC -< pmStreamE
  readEventsFrom4 -< pmStream

Here we have to add type application to handing' so that it knows what is the n and m monads.

I think it would work well if it was HasMonad (HandlingStateT IO) m) but that is less flexible.

Perhaps there is a way around these issues, but if there isn't then it seems that the approach with type classes probably creates more problems than it solves... :-(

turion commented 2 years ago

The trade-off of using Has is that some additional type annotations are necessary as the full type has to be known for the compiler to determine which instance of Has to use. In some cases we don't care about the exception type e, and it is not known as it is thrown away, but with "Has" it has to be known, I think:

throwWhen0
  :: (Monad m, HasExcept () m)
  => Cell m Double Double
throwWhen0 = proc pos ->
  if pos < 0
  then throwC  -< ()
  else returnA -< pos

sineChangeE = do
  try @() $ sine 6 >>> throwWhen0
  try @() $ (constM $ lift $ putStrLn "I changed!")
      >>> throwC
  safe $ sine 10

Here, the exception type has to be explicitly determined (I used type application), which is very inconvenient...

But it should suffice to give sineChangeE a type signature, right?

For HandlingStateT things are more complicated because HandlingStateT m is not really just a monad transformer, but a monad transformer which has type m which has to match the monad inside the transformer (type HandlingStateT m = StateT (HandlingState m) m). For this I created a type class similar to Has but which determines if a whole bottom part of a monad transformer stack is present in a monad stack.

Another way around this might be:

type HasHandling m = HasState (HandlingState m)

This means that the m of HandlingState and the current monad can be different, but that can happen.

class HasMonad (inner :: * -> *) (outer :: * -> *) where
  liftHM :: inner a -> outer a

instance Monad m => HasMonad m m where
  liftHM = id
  {-# INLINE liftHM #-}

instance {-# Overlappable #-} (Monad inner, Monad outer, MonadTrans t, HasMonad inner outer) => HasMonad inner (t outer) where
  liftHM = lift . liftHM
  {-# INLINE liftHM #-}

handling' :: (HasMonad (HandlingStateT n) m, Typeable h, Monad n) => 
  Handle n h -> Cell m arbitrary h
handling' = hoistCell liftHM . handling

The problem here is that the type n is usually unknown. Even activating AmbiguousTypes quickly one gets into a lot or problems. E.g.:

readEventsC :: 
  forall m n arbitrary .
  (MonadIO m, MonadIO n, HasExcept EOLCPortMidiError m, HasMonad (HandlingStateT n) m)
  => String -> Cell m arbitrary [PMEvent]
readEventsC name = proc _ -> do
  pmStreamE <- handling' @n @m $ portMidiInputStreamHandle name -< ()
  pmStream <- exceptC -< pmStreamE
  readEventsFrom4 -< pmStream

Here we have to add type application to handing' so that it knows what is the n and m monads.

I think it would work well if it was HasMonad (HandlingStateT IO) m) but that is less flexible.

Perhaps there is a way around these issues, but if there isn't then it seems that the approach with type classes probably creates more problems than it solves... :-(

Thanks a lot for the thorough investigation!

Well, we don't have to use has-transformers everywhere. Maybe there are many places where it could be applied without problems.

miguel-negrao commented 2 years ago

The trade-off of using Has is that some additional type annotations are necessary as the full type has to be known for the compiler to determine which instance of Has to use. In some cases we don't care about the exception type e, and it is not known as it is thrown away, but with "Has" it has to be known, I think:

throwWhen0
  :: (Monad m, HasExcept () m)
  => Cell m Double Double
throwWhen0 = proc pos ->
  if pos < 0
  then throwC  -< ()
  else returnA -< pos

sineChangeE = do
  try @() $ sine 6 >>> throwWhen0
  try @() $ (constM $ lift $ putStrLn "I changed!")
      >>> throwC
  safe $ sine 10

Here, the exception type has to be explicitly determined (I used type application), which is very inconvenient...

But it should suffice to give sineChangeE a type signature, right?

Unfortunately no, because doing that only binds the last value of the monad it doesn't bind the intermediate values in the do notation which can be of any type.

Will have a look at the other suggestion, thanks !

miguel-negrao commented 2 years ago

Another way around this might be:

type HasHandling m = HasState (HandlingState m)

This means that the m of HandlingState and the current monad can be different, but that can happen.

Ok, lets consider

type HasHandling n m = HasState (HandlingState n) m

But the key functions of HandlingState are handling and handlingParametrised and those require that we are able to run actions in the n monad. But if we use the HasHandling type class we are in the m monad not in the n monad, so we need to lift n to m, otherwise we can't define a generalized handling. Just with the definition above I think there is no way to lift the action in n to m. I think n must be one sublayer of the monad stack (or the bottom monad). In order to define a generalized version of, for instance, readEventsC, we need to have a generalized version of handling, which in turn needs to run actions in n...

Btw, a better name for the type class I proposed would be IsSublayer.

turion commented 2 years ago

I think your HasMonad/IsSublayer class is nearly the same as MonadBase: https://hackage.haskell.org/package/transformers-base

Ok, lets consider

type HasHandling n m = HasState (HandlingState n) m

But the key functions of HandlingState are handling and handlingParametrised and those require that we are able to run actions in the n monad.

Yes, when we call handling, the monads need to be the same. E.g.

provideFoo :: HasHandling SomeFixedM SomeFixedM => Cell SomeFixedM a Foo
provideFoo = handling foo

But we can use this in a general monad:

useFoo :: (HasHandling SomeFixedM m, Has BarT m) => Cell m a b
useFoo = hoistCell liftH provideFoo >>> bar

The monad SomeFixedM will stay the same and never change, even when we lift and hoist m. This makes sense, SomeFixedM is the type of the saved destructors, and they cannot be changed in an obvious way.

I might be wrong somehow, but I think this can work in principle. Maybe a worked out example would be helpful.

miguel-negrao commented 2 years ago

Hi

Yes, when we call handling, the monads need to be the same. E.g.

provideFoo :: HasHandling SomeFixedM SomeFixedM => Cell SomeFixedM a Foo
provideFoo = handling foo

Hum, I don't think I follow here. HasHandling SomeFixedM SomeFixedM will be HasState (HandlingState SomeFixedM) SomeFixedM. This means that SomeFixedM will contain in itself a StateT which contains as state the full SomeFixedM type. So the type contains itself as a layer, is this even possible to define ? This handling above is not the handling from eolc, I assume ?

But we can use this in a general monad:

useFoo :: (HasHandling SomeFixedM m, Has BarT m) => Cell m a b
useFoo = hoistCell liftH provideFoo >>> bar

The monad SomeFixedM will stay the same and never change, even when we lift and hoist m. This makes sense, SomeFixedM is the type of the saved destructors, and they cannot be changed in an obvious way.

I might be wrong somehow, but I think this can work in principle. Maybe a worked out example would be helpful.

I don't think I'm completely following. One thing is that in the current formulation of vivid and portmidi there is no fixed SomeFixedM, it can be any monad as long as it is an instance of MonadIO:

readEventsC
  :: MonadIO m
  => String -> Cell (PortMidiT m) arbitrary [PMEvent]

If we want to keep the same flexibility than we have to deal with an arbitrary (MonadIO n, HasHandling n m).

Perhaps a simple example would be mixing an exception and handling together, e.g.:

test :: Monad m => Cell (ExceptT () (StateT (HandlingState m) m)) () ()
test = proc () -> do
  generalizedHandling (Handle (return ()) (const (return ()))) -< ()
  generalizedThrowC -< ()

generalizeThrowC can be defined as I did above, but the question is how to define generalizedHandling such that this works ?

edit: Actually in the specific case above, generalizedThrowC or current throwC would both work.

miguel-negrao commented 2 years ago

I think your HasMonad/IsSublayer class is nearly the same as MonadBase: https://hackage.haskell.org/package/transformers-base

Thanks, for pointing it out. Indeed, I have known about MonadBase. IsSublayer seems to be a bit more generic, at least in terms of the instances which are given. MonadBase only has specific instances, like IO or []. On the other hand my attempt at IsSublayer often causes "overlapping instances" errors, even with the "overlappable" pragma. The error is usually triggered by something like IsSublayer (t m) (t m).

turion commented 2 years ago

Sorry, I was a bit confused. The HasHandling SomeFixedM SomeFixedM constraint doesn't make any sense, that's right.

We can define this:

provideFoo :: Cell (HandlingStateT SomeFixedM) a Foo
provideFoo = handling foo

But I think it's still true that we can use it in here:

useFoo :: (HasHandling SomeFixedM m, Has BarT m) => Cell m a b
useFoo = hoistCell liftH provideFoo >>> bar

I don't think I'm completely following. One thing is that in the current formulation of vivid and portmidi there is no fixed SomeFixedM, it can be any monad as long as it is an instance of MonadIO:

readEventsC
  :: MonadIO m
  => String -> Cell (PortMidiT m) arbitrary [PMEvent]

Yes, because the portmidi handles are in MonadIO m => m. We could also have:

providePortMidiInput :: MonadIO m => Cell (HandlingStateT (PortMidiT m) a Something` 
providePortMidiInput = handling $ portMidiInputStreamHandle "FooDevice"

usePortMidi :: (MonadIO m, MonadIO n, HasHandlingState m n) => Cell n a Foo
usePortmidi = hoistCell liftH providePortmidiInput >>> foo

Now m and n are independent, as you said. We can lift this and hoist this in all kinds of ways, but the monad m will not be influenced by this.

It's important to note then that when we want to do runHandlingState, we need to make them the same again.

Perhaps a simple example would be mixing an exception and handling together, e.g.:

test :: Monad m => Cell (ExceptT () (StateT (HandlingState m) m)) () ()
test = proc () -> do
  generalizedHandling (Handle (return ()) (const (return ()))) -< ()
  generalizedThrowC -< ()

generalizeThrowC can be defined as I did above, but the question is how to define generalizedHandling such that this works ?

I think this doesn't work in general. At least not without a constraint like MonadBase. Often, we'll be able to use several liftCells, but if the monad is something specific (and not just a transformer) then has-transformers cannot help. I don't really understand what's the best thing to do here.

miguel-negrao commented 2 years ago

Thanks for thinking about this with me ! :-)

Yes, because the portmidi handles are in MonadIO m => m. We could also have:

providePortMidiInput :: MonadIO m => Cell (HandlingStateT (PortMidiT m) a Something` 
providePortMidiInput = handling $ portMidiInputStreamHandle "FooDevice"

usePortMidi :: (MonadIO m, MonadIO n, HasHandlingState m n) => Cell n a Foo
usePortmidi = hoistCell liftH providePortmidiInput >>> foo

Now m and n are independent, as you said. We can lift this and hoist this in all kinds of ways, but the monad m will not be influenced by this.

It's important to note then that when we want to do runHandlingState, we need to make them the same again.

Ok, so I tried putting the code above in Haskell with a few corrections, and I made the types fully explicit so we can see what is going on.

providePortMidiInput :: MonadIO m => Cell (StateT (HandlingState m) m) a (Either EOLCPortMidiError PortMidiInputStream) 
providePortMidiInput = handling $ portMidiInputStreamHandle "FooDevice"

usePortMidi :: forall m n a . (MonadIO m, MonadIO n, HasState (HandlingState m) n) => Cell n a (Either EOLCPortMidiError PortMidiInputStream)
usePortMidi = hoistCell liftH (providePortMidiInput @m)

It doesn't typecheck with

• Couldn't match type ‘n1’ with ‘m’
  ‘n1’ is a rigid type variable bound by
    a type expected by the context:
      forall (n1 :: * -> *). Monad n1 => StateT (HandlingState m) n1 x
    at /home/miguel/Development/Haskell/projects/SC/repos/essence-of-live-coding/essence-of-live-coding-PortMidi/src/LiveCoding/PortMidi.hs:345:25-29
  ‘m’ is a rigid type variable bound by
    the type signature for:
      usePortMidi :: forall (m :: * -> *) (n :: * -> *) a.
                     (MonadIO m, MonadIO n, HasState (HandlingState m) n) =>
                     Cell n a (Either EOLCPortMidiError PortMidiInputStream)
    at /home/miguel/Development/Haskell/projects/SC/repos/essence-of-live-coding/essence-of-live-coding-PortMidi/src/LiveCoding/PortMidi.hs:344:1-141
  Expected type: StateT (HandlingState m) m x -> n x
    Actual type: (forall (n1 :: * -> *).
                  Monad n1 =>
                  StateT (HandlingState m) n1 x)
                 -> n x
• In the first argument of ‘hoistCell’, namely ‘liftH’
  In the expression: hoistCell liftH (providePortMidiInput @m)
  In an equation for ‘usePortMidi’:
      usePortMidi = hoistCell liftH (providePortMidiInput @m)

I'm not sure, but I think that liftH requires that the monad n in t n a -> m a be any type, and not a particular type, and in this case it is fixed to m ?

Perhaps a simple example would be mixing an exception and handling together, e.g.:

test :: Monad m => Cell (ExceptT () (StateT (HandlingState m) m)) () ()
test = proc () -> do
  generalizedHandling (Handle (return ()) (const (return ()))) -< ()
  generalizedThrowC -< ()

generalizeThrowC can be defined as I did above, but the question is how to define generalizedHandling such that this works ?

I think this doesn't work in general. At least not without a constraint like MonadBase. Often, we'll be able to use several liftCells, but if the monad is something specific (and not just a transformer) then has-transformers cannot help. I don't really understand what's the best thing to do here.

That is what I thought too, that is why I tried the MonadBase-like approach, which then gets into other troubles.

miguel-negrao commented 2 years ago

Not sure if this is related to the issue above, but the following gives an error:

x1 :: Monad m => StateT (HandlingState m) m ()
x1 = undefined

x2 :: Monad m => ExceptT () (StateT (HandlingState m) m) ()
x2 = liftH x1
• Overlapping instances for Has
                              t0 (ExceptT () (StateT (HandlingState m) m))
    arising from a use of ‘liftH’
  Matching instances:
    instance [overlappable] [safe] (Monad m, MonadTrans t1, Has t m) =>
                                   Has t (t1 m)
      -- Defined in ‘Control.Monad.Trans.Has’
    instance [safe] Monad m => Has t (t m)
      -- Defined in ‘Control.Monad.Trans.Has’
  (The choice depends on the instantiation of ‘t0, m’
   To pick the first instance above, use IncoherentInstances
   when compiling the other instance declarations)
• In the expression: liftH x1
  In an equation for ‘x2’: x2 = liftH x1

I believe liftH cannot resolve which type class instance to use if the type m in (HandlingState m)is unknown. liftH requires t n a to be polymorphic on n but have t fixed (so it can pick the type class instance). In this case it is polymorphic both on t and n, I believe ?

miguel-negrao commented 2 years ago

Actually, I think the work I had done for my approach to vivid, already solves this issue. The approach I used for MonadState from mtl also works for Has (ST is strict StateT):

transformersToHasState :: (HS.HasState s t, MonadBase m t) => ST.StateT s m a -> t a
transformersToHasState m = do
  currentState <- HS.get
  (a, newState) <- liftBase $ ST.runStateT m currentState
  HS.put newState
  return a

cellGenerelizeHandlingStateT :: (HS.HasState (HandlingState m) t, MonadBase m t) =>
   Cell (HandlingStateT m) a b -> Cell t a b
cellGenerelizeHandlingStateT = hoistCell transformersToHasState

generalizedHandling :: (HS.HasState (HandlingState m) t, MonadBase m t, Typeable h) =>
  Handle m h -> Cell t a h
generalizedHandling = cellGenerelizeHandlingStateT . handling

I think the key trick is the use of MonadBase to reinject the m value back into into t. I think this allows writing generic code (in the sense that the final monad is not hardcoded) which uses HandlingState without having to use explicit lift or liftH. I'm feeling quite happy with this approach :-).

Perhaps I will give it a go at generalizing the code in essence-of-live-coding using this approach.

ps1: In order to use this with PortMidiT the following is needed:

instance (MonadBase b m) => MonadBase b (PortMidiT m) where liftBase = liftBaseDefault

instance Monad m => Has (ST.StateT (HandlingState m)) (PortMidiT m) where
  liftH m = PortMidiT $ liftH m

I haven't quite figured out yet why in the instance above I cannot write liftH = PortMidiT . liftH, although clearly it has to do with how the types are inferred with and without eta reduction.

miguel-negrao commented 2 years ago

I've opened a PR implementing what we discussed here.

miguel-negrao commented 2 years ago

On reddit discussion about something similar to the IsSublayer approach. Interesting.