Add strict over variants #1017

treeowl · 2022-12-21T08:29:14Z

Add over' and iover', which force the result(s) of applying the passed function.

treeowl · 2022-12-23T19:22:31Z

@ekmett and @RyanGlScott , what do you think? Two questions in my mind:

Most things around are marked INLINE. Should these be too?
Should there be operator forms? %~! and %@~!, perhaps? Or would the bangs go in the middle somewhere?

treeowl · 2022-12-23T19:30:22Z

Another question: as @phadej suggested, I wrote these to take specific traversals, rather than ATraversal and AnIndexedTraversal. I don't really understand the conventions around which style to use where.

ekmett · 2022-12-23T21:52:04Z

Combinators can generally ask for any functor that they need, so that's fine.

The main uses for ATraversal/AnIndexedTraversal is to allow users to pass around lists of traversals and the like, and to reconstitute them into real Traversals / IndexedTraversals through cloneTraversal.

ekmett · 2022-12-23T21:56:13Z

These should be marked INLINE, and the current morphology would likely lead to %!~ and %!@~ probably, (and %!= and %!@= respectively for state). The nice thing about the state one is you can use the usual strict function over a monad encoding I think using <$!>.

phadej · 2022-12-23T22:01:49Z

src/Control/Lens/Traversal.hs

+-- fields.
+--
+-- @
+-- over traverse (const ⊥) [1,2] = [⊥, ⊥]


These can be turned into proper doctests. length $ ... and matching like

... ...Exception... ...

for an error. It's not the prettiest, but it's an honest test.

To my mind doctests wouldn't do as good a job of documenting, but I can write them anyway. How do I run those locally?

cabal build all --enable-tests cabal-docspec

To my mind doctests wouldn't do as good a job of documenting, but I can write them anyway.

There is barely any non-doctest examples in lens. They may do less good job as documenting, but at least they stay up to date. There are hundreds of examples in lens package. No-one would ever go through them manually and verify they still type-check/work/etc.

I'm getting errors from cabal-docspec about mismatched GHC and plan versions. Unfortunately, cabal-docspec doesn't seem to have much in the way of documentation, so I can't figure out where to go from there.

src/Control/Lens/Traversal.hs

treeowl · 2022-12-23T23:15:52Z

These should be marked INLINE, and the current morphology would likely lead to %!~ and %!@~ probably, (and %!= and %!@= respectively for state). The nice thing about the state one is you can use the usual strict function over a monad encoding I think using <$!>.

Are you suggesting traversing in the ambient monad? Or just pattern matching out of Solo into it?

ekmett · 2022-12-24T01:06:22Z

It's a state monad variant, so in theory the <$!> trick should be usable in the ambient monad. Not wedded to doing it that way, was just indicating it's probably doable and quite clean.

phadej · 2022-12-24T01:22:45Z

src/Control/Lens/Setter.hs

@@ -342,6 +342,10 @@ cloneIndexedSetter l pafb = taintedDot (runIdentity #. l (Indexed $ \i -> Identi
 -- >>> over _1 show (10,20)
 -- ("10",20)
 --
+-- Like 'fmap', @over@ is normally lazy in the result(s) of calling the


I'm also on the edge with this "warning".

Shouldn't the strict version also have a similar warning that it actually forces all the computations up front. I.e. over' traverse f aList is most likely a wrong thing to do. (And the examples used are indeed contrived).

Yes, they're contrived. Should I give a datatype definition and makeLenses and all that jazz? A more realistic case might be a record with a lazy field. If you use over several times, you'll build a thunk chain in there. over' traverse isn't too far from what, say, Data.Map.Strict.map does, though it's probably less efficient.

That's a problem (to give an non contrived examples). I think in majority of relevant cases you'd rather make the field strict. In fact, there are people which would prefer Data.Map.Strict to be a separate datatype. (The type system doesn't help to keep the Map thunk free.)

i.e. while over' is useful, I don't think it should be commonly used.

The only case I can think out of my head is Seq. Where over' traverse doesn't leave thunks, but AFAIK ruins asymptotics.

@phadej over' could quite reasonably be used with ix for Array, SmallArray (if that has an instance), Vector, Seq, Map, or IntMap. I imagine some folks just use at instead, but that's semantically overkill (and potentially expensive) for just forcing.

phadej · 2022-12-24T10:06:37Z

This "solves" #944

treeowl · 2022-12-26T02:29:16Z

It occurs to me that there's another approach to this issue, and it's not entirely clear whether one or the other is better. First, an exceedingly simple monad transformer that probably belongs in transformers:

newtype SoloT m a = SoloT
  { runSoloT :: m (Solo a) }

-- Note: in the below instances, clever implementations of <$,
-- <*, *>, or >> will almost certainly be wrong; the defaults are
-- correct.
instance Monad m => Functor (SoloT m) where
  fmap = liftM
instance Monad m => Applicative (SoloT m) where
  pure = SoloT . pure . Solo
  liftA2 = liftM2
  (<*>) = ap
instance Monad m => Monad (SoloT m) where
  m >>= f = SoloT $ runSoloT m >>= \(Solo a) -> runSoloT (f a)
instance MonadTrans SoloT where
  lift = SoloT . fmap Solo

Now, we can strictify optics in a monadic context:

strictly :: Monad m => LensLike (SoloT m) s t a b -> LensLike m s t a b
strictly l f s = runSoloT (l (\a -> SoloT $ f a >>= \ !b -> pure $ Solo b) s) >>= \(Solo t) -> pure t

In particular, we can write

overly' :: LensLike (SoloT Identity) s t a b -> (a -> b) -> s -> t
overly' l = over (strictly l)

I still think a direct implementation of over' is better for its exact purpose, but maybe strictly is also a good thing to have on hand?

treeowl · 2022-12-26T07:05:04Z

strictly seems stricter, actually, which may be a bad thing. Let's stick with over' stuff for now, and think through the rest later.

treeowl · 2022-12-26T08:46:56Z

Sorry, no it's not. This stuff is confusing! Let's consider both.

treeowl · 2022-12-26T20:41:34Z

Ah, now I understand why that felt off. The monad transformer is the wrong thing, but there's a very similar (slightly lazier) applicative transformer that I believe does exactly what I mean.

type BoxT :: (Type -> Type) -> Type -> Type
newtype BoxT m a = BoxT
  { runBoxT :: m (Solo a) }
  deriving (Functor)

instance Apply m => Apply (BoxT m) where
  liftF2 f (BoxT m) (BoxT n) = BoxT (liftF2 (liftA2 f) m n) 
  {-# INLINE liftF2 #-}
instance Applicative m => Applicative (BoxT m) where
  pure = BoxT . pure . Solo
  {-# INLINE pure #-} 
  BoxT m <*> BoxT n = BoxT (liftA2 (<*>) m n)
  {-# INLINE (<*>) #-}
#if MIN_VERSION_base(4,10,0)
  liftA2 f (BoxT m) (BoxT n) = BoxT (liftA2 (liftA2 f) m n)
  {-# INLINE liftA2 #-}
#endif

Now, we can be exceedingly general:

strictly :: (Functor m, Profunctor p, Profunctor q) => Optical p q (BoxT m) s t a b -> Optical p q m s t a b
strictly l f = rmap (fmap getSolo .# runBoxT) $ l (rmap (\mb -> BoxT ((Solo $!) <$> mb)) f)
{-# INLINE strictly #-}

treeowl · 2022-12-26T23:22:49Z

src/Control/Lens/Traversal.hs

+-- 'Setter'.
+newtype BoxT m a = BoxT
+  { runBoxT :: m (Solo a) }
+  deriving (Functor, Foldable, Traversable)


Where should this type be defined and exported?

So you don't want to wait until SoloT is added somewhere, as you proposed in private mail to me and Ross (maintainer of transformers)?

I understand that you have time to work on stuff during holidays, but I honestly don't, so will politely ignore you until the next year.

SoloT turned out to be the wrong sort of transformer for this purpose. I made a mistake and changed course.

Please tell that to others in that email message too. I wasn't aware that I don't need to think about that anymore.

treeowl · 2022-12-27T02:11:19Z

I don't know how much to overgeneralize things. We could, for example, write

over' :: (Profunctor p, Profunctor q) => Optical p q Solo s t a b -> p a b -> q s t
over' l f = rmap getSolo $ l (rmap (Solo $!) f)

Is that more useful or more confusing?

phadej

This pull request has overgrown what I feel comfortable to comment about. I'd ask @ekmett to review this.

phadej · 2022-12-27T12:34:49Z

I don't know how much to overgeneralize things. We could, for example, write
over' :: (Profunctor p, Profunctor q) => Optical p q Solo s t a b -> p a b -> q s t
over' l f = rmap getSolo $ l (rmap (Solo $!) f)
Is that more useful or more confusing?

Will over' traverse (+1) [1,2,3] still work?

treeowl · 2022-12-27T12:42:27Z

I don't know how much to overgeneralize things. We could, for example, write
over' :: (Profunctor p, Profunctor q) => Optical p q Solo s t a b -> p a b -> q s t
over' l f = rmap getSolo $ l (rmap (Solo $!) f)
Is that more useful or more confusing?
Will over' traverse (+1) [1,2,3] still work?

Yes. p and q both specialize to ->, so rmap becomes (.).

phadej · 2022-12-27T12:52:34Z

So can over be of Optical p q Identity s t a b -> p a b -> q s t form? I'd say it's both or neither.

treeowl · 2022-12-27T13:09:33Z

In that scheme, over would become

over :: (Profunctor p, Profunctor q) => Optical p q Identity s t a b -> p a b -> q s t
over l f = runIdentity #. l (Identity #. f)

treeowl · 2022-12-28T04:31:06Z

It occurs to me that when used in a non-trivial functor, the traversal produced by strictly will always leave thunks in it. Only forcing those thunks from the outside will actually force the targets. This can be avoided if the ambient functor is either a Traversable or a Monad:

strictlyT :: (Traversable f, Profunctor p, Profunctor q) => Optical p q (BoxT f) s t a b -> Optical p q f s t a b
strictlyT l f = rmap (getSolo . sequenceA .# runBoxT) $ l (rmap (BoxT #. fmap (Solo $!)) f)

strictlyM :: (Monad f, Profunctor p, Profunctor q) => Optical p q (BoxT f) s t a b -> Optical p q f s t a b
strictlyM l f = rmap ((>>= \(Solo r) -> pure r) .# runBoxT) $ l (rmap (BoxT #. fmap (Solo $!)) f)

Is either of these worth having?

* Add `strictly` to turn a lazy (standard) traversal into a strict one that forces targets before installing them. * Add `over'`, `iover'`, `modifying'`, `imodifying'`, and corresponding operators `%!~`, `%!@~`, `%!=`, and `%!@=`. * Adjust documentation. Closes ekmett#1016

ekmett · 2022-12-28T08:31:49Z

i think they are worth having, i hate the names, but not the functionality.

treeowl · 2022-12-28T08:37:31Z

@ekmett should the monadic version use the SoloT monad transformer instead of the BoxT applicative transformer? My guess is yes, but I probably need to play with some benchmarks.

phadej reviewed Dec 23, 2022

View reviewed changes

phadej reviewed Dec 24, 2022

View reviewed changes

treeowl commented Dec 26, 2022

View reviewed changes

treeowl force-pushed the strict-over branch 3 times, most recently from 5083a08 to b1be2ff Compare December 27, 2022 00:37

treeowl force-pushed the strict-over branch 10 times, most recently from ab7c984 to 4eab46d Compare December 27, 2022 04:37

treeowl requested a review from phadej December 27, 2022 04:45

treeowl mentioned this pull request Dec 27, 2022

Are state modifications supposed to be so lazy? #1018

Open

phadej reviewed Dec 27, 2022

View reviewed changes

treeowl force-pushed the strict-over branch 5 times, most recently from 1b0e3dd to 9de9c7c Compare December 28, 2022 03:59

treeowl mentioned this pull request Dec 28, 2022

Restrict over', iover', and set' to traversals well-typed/optics#475

Open

treeowl force-pushed the strict-over branch from 9de9c7c to 2cab272 Compare December 28, 2022 04:23

treeowl force-pushed the strict-over branch from 2cab272 to 9d981ee Compare December 28, 2022 04:34

Strictly variants

87f36ca

Add strict over variants #1017

Are you sure you want to change the base?

Add strict over variants #1017

Conversation

treeowl commented Dec 21, 2022

treeowl commented Dec 23, 2022

treeowl commented Dec 23, 2022

ekmett commented Dec 23, 2022

ekmett commented Dec 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phadej Dec 24, 2022 • edited

Choose a reason for hiding this comment

treeowl Dec 26, 2022 • edited

Choose a reason for hiding this comment

treeowl commented Dec 23, 2022

ekmett commented Dec 24, 2022

phadej Dec 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phadej Dec 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

treeowl Dec 26, 2022 • edited

Choose a reason for hiding this comment

phadej commented Dec 24, 2022 • edited

treeowl commented Dec 26, 2022 • edited

treeowl commented Dec 26, 2022

treeowl commented Dec 26, 2022

treeowl commented Dec 26, 2022 • edited

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

treeowl commented Dec 27, 2022

phadej left a comment

Choose a reason for hiding this comment

phadej commented Dec 27, 2022

treeowl commented Dec 27, 2022

phadej commented Dec 27, 2022

treeowl commented Dec 27, 2022

treeowl commented Dec 28, 2022

ekmett commented Dec 28, 2022

treeowl commented Dec 28, 2022

phadej Dec 24, 2022 •

edited

treeowl Dec 26, 2022 •

edited

phadej Dec 24, 2022 •

edited

phadej Dec 24, 2022 •

edited

treeowl Dec 26, 2022 •

edited

phadej commented Dec 24, 2022 •

edited

treeowl commented Dec 26, 2022 •

edited

treeowl commented Dec 26, 2022 •

edited