sturdy icon indicating copy to clipboard operation
sturdy copied to clipboard

Stackage

Open fakedrake opened this issue 5 years ago • 16 comments

Could we get a package out of this? I like your code on arrows

fakedrake avatar May 31 '20 17:05 fakedrake

Thanks for checking out my library. Yes, this sounds like a good idea. @seba--, what do you think?

I'm trying to decide if I should split this library into smaller packages or if I should upload it as a whole. @fakedrake, which part of the library would you use? Would you use the library for implementing static analyses or would you only use the arrow code for implementing other applications?

svenkeidel avatar Jun 02 '20 12:06 svenkeidel

The overall issue I am facing is that there is no arrow transformer library on stackage so I have to re-implement everything. Which sucks. So a package with everything under Control.Arrow would be perfect.

fakedrake avatar Jun 02 '20 13:06 fakedrake

Makes sense. Well, there is the arrow transformer library atl. Have you tried atl and is there a problem with it?

Furthermore, currently half of whats in sturdy:Control.Arrow is specific to static analyses and this is probably of no use for the day to day developer. What would be of use are the general purpose transformers ReaderT, WriterT, StateT, ConstT, StaticT, ContT, KleisliT, and CoKleisliT. These transformers could be extract into their own package, however, I need to be convinced that this is worth the effort. Maybe it is worth to contribute some of this code to the atl project.

svenkeidel avatar Jun 02 '20 13:06 svenkeidel

Here is a list of all the files that are general purpose and reusable. Now you just need to remove missing imports and type class instances. I removed the ExceptT-like transformers because they include some details specific to static analysis.

svenkeidel avatar Jun 02 '20 13:06 svenkeidel

The list looks legit. TBH I only shamelessly copied KleisliT, StateT, ReaderT and ContT (so far). Better than atl is arrows but it's not on stackage, it seems discontinued, it's missing important transformers (ehm KleisliT), and most importantly I like the implementations in sturdy much better.

fakedrake avatar Jun 02 '20 14:06 fakedrake

Thanks. We put quite some work into the arrow code to improve the generated code and its performance.

Ok, maybe its time for a new arrow library. I see what I can do.

svenkeidel avatar Jun 03 '20 06:06 svenkeidel

I think this is a great idea.

seba-- avatar Jun 03 '20 07:06 seba--

@fakedrake, you can follow the current progress at https://github.com/svenkeidel/sturdy/tree/extract-arrow-transformer-library.

svenkeidel avatar Jun 03 '20 10:06 svenkeidel

Allright, I extracted the library and added some minimal documentation. @fakedrake, can you give me some feedback on the library in the arrows folder please? In particular:

  • Is documentation missing/confusing?
  • Are any of the type class and function names confusing? Are there any type class methods missing?

I would greatly appreciate any contributions in terms of feedback, documentation and code. Thanks.

svenkeidel avatar Jun 03 '20 16:06 svenkeidel

Wow you did quite a bit of work, thanks! Anyway documentation looks legit. Thanks for moving some benchmarks in there too.

What's up with changing around ArrowTrans and ArrowLift?

fakedrake avatar Jun 04 '20 17:06 fakedrake

This is low priority by I like Automaton, if you ever get the chance to add it. This implementation has better naming and implementation (IMHO) but it's not a transformer.

fakedrake avatar Jun 04 '20 17:06 fakedrake

What's up with changing around ArrowTrans and ArrowLift ?

I swapped the names to make the name of ArrowTrans more consistent with mtl's MonadTrans. The ArrowTrans.lift' function is the arrow equivalent of MonadTrans.lift.

This is low priority by I like Automaton

Sure I see what I can do. I have never used this transformer. What is it useful for?

svenkeidel avatar Jun 04 '20 19:06 svenkeidel

One example might be circuits. I have seen in for stream processing but I can't find the citation right now.

fakedrake avatar Jun 04 '20 20:06 fakedrake

I saw you added it, thanks a lot! Final request and I will leave you alone: could you move FailureT in there as well? Edit: FailureT inherits everything from KleisliT but it doesn't really need ArrowMonad for ... well everything, just ArrowChoice. I will write it up when I get the chance but here is a very rough sketch of what I propose:

instance (ArrowChoice c) => Category (KleisliT Maybe c) where
  KleisliT f . KleisliT g = KleisliT $ g
    >>> arr (maybe (Left ()) Right)
    >>> (arr $ const Nothing) ||| f
  id = KleisliT $ arr return . id

instance (Category (KleisliT f c),Arrow c,Monad f) => Arrow (KleisliT f c) where
  arr = arrLift . arr
  first (KleisliT f) = KleisliT $ first f >>> arr (\(fc,u) -> (,u) <$> fc)

fakedrake avatar Jun 05 '20 12:06 fakedrake

I see. So you want to eliminate the ArrowMonad class and instead have a Category Kleisli instance vor every Monad. I like this idea.

About adding FailureT: the only problem I have is that the ArrowFail class is too general due to it's use for static analysis. Either we use this more general version or introduce a simpler version without the Join constraint.

svenkeidel avatar Jun 06 '20 22:06 svenkeidel

Sorry, I am realizing that what I wrote was not clear at all. What I mean is this:

newtype ExceptT e c a b = ExceptT (c a (Either e b))
instance ArrowChoice c => Category (ExceptT e c) where
  ExceptT f . ExceptT g = ExceptT $ g >>> arr Left ||| f
  id = ExceptT $ arr return . id

instance ArrowChoice c => Arrow (ExceptT f c) where
  arr f = ExceptT $ arr $ return . f
  first (ExceptT f) = ExceptT $ first f >>> arr (\(fc,u) -> (,u) <$> fc)

The problem is that as far as I can tell, the only ArrowMonad is KleisliT and the arrows implemented as such, but almost all the arrows in the package are ArrowChoice.

I know performance is important for sturdy so disregard this if this change affects performance badly.

fakedrake avatar Jun 07 '20 17:06 fakedrake