Binary nondeterminism #197

robrix · 2019-08-25T09:06:00Z

This PR redefines NonDetC, changing it from a Church-encoded list to a Church-encoded binary tree. This should result in greater preservation of parallelism between branches, tho I haven’t actually measured that. It should also result in slightly simpler Core since e.g. pure no longer needs to invoke both continuations, but again, I haven’t measured that.

robrix

Ready for review.

robrix · 2019-09-03T15:16:24Z

src/Control/Effect/Cull.hs

+  l <|> r = CullC $ ReaderC $ \ cull ->
+    if cull then
+      NonDetC $ \ fork leaf nil ->
+        runNonDetC (runReader cull (runCullC l)) fork leaf (runNonDetC (runReader cull (runCullC r)) fork leaf nil)


When we’re inside a cull, we alternate by using the rhs as the nil continuation for the lhs.

robrix · 2019-09-03T15:16:33Z

src/Control/Effect/Cull.hs

+      NonDetC $ \ fork leaf nil ->
+        runNonDetC (runReader cull (runCullC l)) fork leaf (runNonDetC (runReader cull (runCullC r)) fork leaf nil)
+    else
+      runReader cull (runCullC l) <|> runReader cull (runCullC r)


When we aren’t inside a cull, then we just alternate as normal for the underlying NonDetC.

robrix · 2019-09-03T15:18:43Z

src/Control/Effect/Cut.hs

  {-# INLINE fail #-}

 instance MonadFix m => MonadFix (CutC m) where
  mfix f = CutC (\ cons nil _ -> mfix (\ a -> runCutC (f (head a)) (fmap . (:)) (pure []) (pure [])) >>= foldr cons nil)
  {-# INLINE mfix #-}

 instance MonadIO m => MonadIO (CutC m) where
-  liftIO io = CutC (\ cons nil _ -> liftIO io >>= flip cons nil)
+  liftIO io = lift (liftIO io)


I attempted to apply the same approach to CutC, but (much to my surprise) the semantics of cut/call are such that you actually want the left-to-right sequencing of the encoded list formulation. I’m not yet convinced that it can’t be done with a binary tree formulation, but it sure isn’t straightforward.

Can you walk me through what’s going on here? I’m not sure I follow the operative difference introduced by this change.

The semantics are unchanged, because:

lift m = CutC (\ cons nil _ -> m >>= flip cons nil)

thus,

lift (liftIO io) = CutC (\ cons nil _ -> liftIO io >>= flip cons nil)

which is exactly the definition we’ve replaced.

robrix · 2019-09-03T15:19:37Z

src/Control/Effect/NonDet.hs

+  deriving (Functor, Generic1)
+
+instance HFunctor NonDet
+instance Effect   NonDet


I honestly don’t think we should use DeriveAnyClass for these, as it’s a false economy.

Up to you. I personally like it, but given that it’s a two-line difference I’m not worked up about it.

robrix · 2019-09-03T15:20:06Z

src/Control/Effect/NonDet.hs



 -- | Run a 'NonDet' effect, collecting all branches’ results into an 'Alternative' functor.
 --
--   Using '[]' as the 'Alternative' functor will produce all results, while 'Maybe' will return only the first. However, unlike 'runNonDetOnce', this will still enumerate the entire search space before returning, meaning that it will diverge for infinite search spaces, even when using 'Maybe'.
+--   Using @[]@ as the 'Alternative' functor will produce all results, while 'Maybe' will return only the first. However, unlike 'Control.Effect.Cull.runNonDetOnce', this will still enumerate the entire search space before returning, meaning that it will diverge for infinite search spaces, even when using 'Maybe'.


Haddock can’t link to [] because [] isn’t defined in (Haskell) source. Syntax sugar!

robrix · 2019-09-03T15:26:42Z

src/Control/Effect/NonDet.hs

  {-# INLINE liftIO #-}

 instance MonadPlus (NonDetC m)

 instance MonadTrans NonDetC where
-  lift m = NonDetC (\ cons nil -> m >>= flip cons nil)
+  lift m = NonDetC (\ _ leaf _ -> m >>= leaf)


We no longer invoke the nil continuation for pure & lift, which should make this slightly more efficient in general.

robrix · 2019-09-03T15:27:39Z

src/Control/Effect/NonDet.hs

+
+instance Alternative BinaryTree where
+  empty = Nil
+  (<|>) = Fork


We need the Alternative instance because runNonDet (used in eff) requires it. cf #207.

robrix · 2019-09-03T15:32:03Z

src/Control/Effect/NonDet.hs

  {-# INLINE eff #-}


+data BinaryTree a = Nil | Leaf a | Fork (BinaryTree a) (BinaryTree a)
+  deriving (Eq, Foldable, Functor, Ord, Show, Traversable)


I’ve opted not to export this for the time being, but I feel a bit weird about that. I don’t know that I really want to add a Data.BinaryTree module, either, since that seems like a potentially contended name for the module. Tho perhaps I worry too much, since hoogle doesn’t seem to know about any such modules?

I don’t object to keeping it hidden, personally. The more details we hide, the more we can iterate on the internals without breaking public API. We’re already entailing a good bit of churn these days, after all!

That is an excellent point.

robrix · 2019-09-03T15:35:33Z

src/Control/Effect/NonDet.hs

+
+instance Applicative BinaryTree where
+  pure = Leaf
+  f <*> a = fold Fork (<$> a) Nil f


This and the definition of >>= make me extremely happy.

robrix · 2019-09-03T15:36:07Z

src/Control/Effect/NonDet.hs

+  (<|>) = Fork
+
+instance Monad BinaryTree where
+  a >>= f = fold Fork f Nil a


>>= on trees is substitution.

patrickt · 2019-09-08T18:11:20Z

Informal first observations based off of #211 indicate this gives a slight speedup, which is further improved by marking the BinaryTree Functor/Monad/etc. methods as INLINE. ✨

robrix · 2019-09-08T23:56:34Z

That’s super exciting!

patrickt · 2019-09-09T20:09:32Z

src/Control/Effect/Cull.hs

@@ -51,18 +52,20 @@ cull m = send (Cull m pure)
 --
 --   prop> run (runNonDet (runCull (pure a <|> pure b))) === [a, b]
 runCull :: Alternative m => CullC m a -> m a
-runCull (CullC m) = runNonDetC (runReader False m) ((<|>) . pure) empty
+runCull (CullC m) = runNonDetC (runReader False m) (<|>) pure empty


I like that this got simpler.

patrickt · 2019-09-09T20:10:56Z

src/Control/Effect/NonDet.hs

+  deriving (Functor, Generic1)
+
+instance HFunctor NonDet
+instance Effect   NonDet


Up to you. I personally like it, but given that it’s a two-line difference I’m not worked up about it.

src/Control/Effect/NonDet.hs

patrickt · 2019-09-09T20:14:44Z

src/Control/Effect/Cut.hs

  {-# INLINE fail #-}

 instance MonadFix m => MonadFix (CutC m) where
  mfix f = CutC (\ cons nil _ -> mfix (\ a -> runCutC (f (head a)) (fmap . (:)) (pure []) (pure [])) >>= foldr cons nil)
  {-# INLINE mfix #-}

 instance MonadIO m => MonadIO (CutC m) where
-  liftIO io = CutC (\ cons nil _ -> liftIO io >>= flip cons nil)
+  liftIO io = lift (liftIO io)


Can you walk me through what’s going on here? I’m not sure I follow the operative difference introduced by this change.

robrix · 2019-09-09T21:04:56Z

patrickt

Wonderful! One last nitpick: I would mark the changelog entry for this patch as backwards-compatible. Otherwise, well done! ❤️

robrix · 2019-09-11T01:42:48Z

I would mark the changelog entry for this patch as backwards-compatible.

The type of NonDetC & runNonDetC changed, and both are exported, so technically it’s not, alas.

robrix added 30 commits August 22, 2019 13:41

Lift MonadFail/MonadIO using MonadTrans.

8edd680

Test that empty is handled correctly within cull.

eb45dda

Fix the doc reference to runNonDetOnce.

91be10b

Fix the doc reference to [].

162d626

Add some simple tests of Applicative.

1900a53

Add a simple test of Alternative.

f3c830f

Define a datatype of binary trees.

83cbf16

Derive some instances for B.

c80d652

Define an Applicative instance for B.

991e55c

Define an Alternative instance for B.

34a9979

Define a Monad instance for B.

8359300

Rename B to Bin.

6bd09c1

Derive some more instances for Bin.

5ec21db

Export Bin.

ae79dad

Define a fold over Bin.

a37b26e

Test that NonDetC preserves branching structure.

b72a107

Define NonDetC as a church-encoded binary tree.

c53a119

Don’t prime the names in mfix.

20bdb4b

Simplify the MonadFail & MonadIO instances for CutC using MonadTrans.

42bdf1b

Simplify the MonadTrans instance for NonDetC.

cf3db93

Stub in a module for binary trees.

5ae55a2

Move Bin to its own module.

8d0cf59

Rename Bin to BinaryTree & foldBin to fold.

acadd3b

Spacing.

6460e72

Alignment.

b8f9b15

Alignment.

99b748f

Move BinaryTree back into Control.Effect.NonDet but don’t export it.

d2bbd2d

🔥 a redundant use of GeneralizedNewtypeDeriving.

73a8ba3

🔥 uses of DeriveAnyClass & DerivingStrategies.

725cec1

Spacing.

dd85cf0

This was referenced Aug 25, 2019

Split NonDet into Empty & Choose effects #199

Merged

Non-empty nondeterminism effect #198

Merged

This was referenced Sep 1, 2019

Separate carrier and effect modules #204

Merged

Base instances #206

Merged

Simplified eliminators #207

Merged

Merge branch 'master' into binary-nondet

a7db05e

robrix mentioned this pull request Sep 3, 2019

NonDetC’s MonadFix instance shares the constructed value between all branches #209

Closed

robrix added 4 commits September 3, 2019 10:48

Try to be consistent with eliminator names.

5674d9b

Don’t shadow pure.

0a08f49

Define <*> using fold.

e70be43

Define >>= using fold.

3eabaf1

robrix commented Sep 3, 2019

View reviewed changes

robrix marked this pull request as ready for review September 3, 2019 15:36

robrix and others added 2 commits September 3, 2019 11:39

Add a note to the changelog.

06dbd73

Merge branch 'master' into binary-nondet

069c9cf

robrix mentioned this pull request Sep 8, 2019

Benchmark nondeterminism carriers #210

Closed

patrickt requested changes Sep 9, 2019

View reviewed changes

robrix and others added 2 commits September 9, 2019 16:33

INLINE all the things.

1bb5299

Merge branch 'master' into binary-nondet

d4e1967

robrix requested a review from patrickt September 9, 2019 20:35

Merge branch 'master' into binary-nondet

56d00bf

robrix added this to the 0.6 milestone Sep 10, 2019

patrickt approved these changes Sep 11, 2019

View reviewed changes

robrix merged commit 963ad9f into master Sep 11, 2019

robrix deleted the binary-nondet branch September 11, 2019 01:43

robrix mentioned this pull request Oct 23, 2019

Eliminate binary trees #286

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Binary nondeterminism #197

Binary nondeterminism #197

robrix commented Aug 25, 2019

robrix left a comment

robrix Sep 3, 2019

robrix Sep 3, 2019

robrix Sep 3, 2019

patrickt Sep 9, 2019

robrix Sep 9, 2019

robrix Sep 3, 2019

patrickt Sep 9, 2019

robrix Sep 3, 2019

robrix Sep 3, 2019

robrix Sep 3, 2019

robrix Sep 3, 2019

patrickt Sep 8, 2019

robrix Sep 9, 2019

robrix Sep 3, 2019

robrix Sep 3, 2019

patrickt commented Sep 8, 2019

robrix commented Sep 8, 2019

patrickt Sep 9, 2019

patrickt Sep 9, 2019

patrickt Sep 9, 2019

robrix commented Sep 9, 2019

patrickt left a comment

robrix commented Sep 11, 2019

Binary nondeterminism #197

Binary nondeterminism #197

Conversation

robrix commented Aug 25, 2019

robrix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickt commented Sep 8, 2019

robrix commented Sep 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robrix commented Sep 9, 2019

patrickt left a comment

Choose a reason for hiding this comment

robrix commented Sep 11, 2019