Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Break early" from monad CE #135

Closed
cmeeren opened this issue Oct 31, 2018 · 37 comments
Closed

"Break early" from monad CE #135

cmeeren opened this issue Oct 31, 2018 · 37 comments

Comments

@cmeeren
Copy link
Contributor

cmeeren commented Oct 31, 2018

I have a result CE in Cvdm.ErrorHandling for monadic error handling. For the example below, sideEffect1 and sideEffect2 are never invoked, since do! binds an error (I think also Delay is involved, but I don't have a clear grasp of CE builders).

let _ = result {
  if true then
    do! Error err
    sideEffect1 ()
  sideEffect2 ()
}

However, if I replace result with monad (or monad.fx or others, I've tried them all), the side effects are invoked.

Is it possible to get my desired behavior using a generic CE from FSharpPlus?

@gusty
Copy link
Member

gusty commented Oct 31, 2018

If you replace it with monad or monad.fx (both are equivalent since .fx is the default) it will only execute the second side effect, unless you use the else keyword.

This as weird as it seems, it's a bit standard in Computation Expressions, actually last week someone asked at work practically the same question over async workflows:

let threatLevel = 1
async {
  if threatLevel < 5 then
    return ()
  System.Console.WriteLine("Launch missiles")
} |> Async.RunSynchronously

It launch, unless you use else.

@cmeeren
Copy link
Contributor Author

cmeeren commented Oct 31, 2018

Oh, my mistake then about sideEffect1. Do you know where I can read more about this behavior regarding the async CE?

Also, there's other weird behavior. Consider this:

use enum = ([0] |> Seq.ofList).GetEnumerator()

let result = monad {
  while enum.MoveNext () do
    ignore enum.Current
}

This fails on enum.Current with System.InvalidOperationException : Enumeration has not started. Call MoveNext. But how is it possible to have arrived at the enum.Current line without MoveNext having been called?

@gusty
Copy link
Member

gusty commented Oct 31, 2018

I don't have enough context to compile your code (monad is not inferred).
Could you post a full example?

@cmeeren
Copy link
Contributor Author

cmeeren commented Oct 31, 2018

Does the monad type matter? Try let result : Result<unit, unit> = ... or anything else that compiles. Let me know if you can't repro and I'll have a look at it.

@gusty
Copy link
Member

gusty commented Oct 31, 2018

No it shouldn't matter. What does matter here is the type of CE (lazy/strict).

If you use the strict one (monad.strict) it will work as expected.

The offending commit is this one: 4d07943#diff-8ed049da6bf12bafde3fe125f70862fb

I was between two worlds here.
Without this commit, if you use a delayed computation (which is the default) in place of a strict one, it would always fails to compile.

But I wanted to be smarter and found a way to "autosense" the kind of computation, it works in most cases, but in some others (like this one) it will throw a runtime error.

I'm not 100% convinced if it was the best decision.
Or maybe there's a way to improve the autosense for these cases.

@cmeeren
Copy link
Contributor Author

cmeeren commented Oct 31, 2018

I've got no idea about this auto-sensing, but do you agree that the while enum.MoveNext() behavior is a bug, or are you saying it's expected behavior? If so, I'll be very wary of ever using monad since it evidently can lead to unexpected runtime exceptions for code that compiles and looks fine.

@gusty
Copy link
Member

gusty commented Oct 31, 2018

It's not a bug, in the sense that I can understand and explain why does it happens. And it will fail immediately and systematically in the first iteration, still it's a runtime error which is not that great.

Moreover, it wasn't unexpected to me, when you first posted it I was 99% sure that was the reason, just wanted to reproduce it to make sure.

We can discuss and eventually decide to revert the auto-sense and convert that scenario to a compile-time error.

I think the problem is that understanding Computation Expressions is not straight-forward, because you need at some point to understand how the code is desugared and how it's executed. My guess is that understanding 4 main CE's models is worth the effort in place of understanding the model of every single custom CE in the wild.

On top of that, we can certainly add more documentation to those 4 generic models, they definitely deserve it.

Some additional tests are very beneficial and so is this discussion, I'm happy that you are digging into this as we can still decide what's the best approach, and eventually rectify some decisions.

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 1, 2018

I completely agree that thorough documentation on the four generic CEs would be prudent. :) As you say, CEs can and do bite, and understanding how they are desugared and run can be far from trivial even for experienced F# devs. I'll certainly stay away from these four CEs until they are extensively documented; I won't risk runtime exceptions for reasons I can't explain on my own and that aren't documented just for added convenience.

@wallymathieu
Copy link
Member

diagram
Source
?

@gusty
Copy link
Member

gusty commented Nov 1, 2018

@cmeeren Just in case you are interested, I can give you more details of what's the reason.

In short, all monads (or CEs) can be made strict, but not all monads are lazy by nature.

This contradicts a bit the design decision of using the lazy one as default, which was motivated by the fact that the lazy one was more interesting.

As a result, one nice thing is that in most cases you don't need to explicitly mark your CE's as strict, in most case they will work, unless they use an enumerator (apart from the enumerator, there were other scenarios where it would fail but the "auto-sense" solved those ones).

So, if a CE makes use of an enumerator it has to either:

  • By a lazy computation, with a lazy Delay function implemented
  • Be explicitly marked as strict

If none of these 2 conditions are met, you'll get a run-time error in the very first iteration.

Again, this run-time error is expected in the sense that you are forcing a delayed construct to iterate but the delay function is not lazy, so that won't work since it's not really delaying anything at all.

That's for instance the reason why, ReaderT and StateT monad transformers have a Delay function implemented.

Now, if you don't agree with this design, I would be happy to discuss what the alternatives are.

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 1, 2018

Thank you for the explanation! I really appreciate it.

I am very fuzzy on the whole lazy vs. strict thing; I don't really understand exactly which parts of CEs are lazy or not, nor exactly how the Delay function impacts this. When I created Cvdm.ErrorHandling, I very much relied upon the unit tests to verify the behavior I expected, and hammered at the CE builder implementation until the tests were green.

Please take a look at the unit tests for the result builder, which show the behavior I expect. Perhaps that can clarify what I myself cannot. Please do let me know if you disagree with the behavior I have implemented; I'm very eager to discuss and learn.


As for what you mention about enumerators: Are you saying that in the very specific case of having an enumerator where you call MoveNext in a while loop, a CE behaves differently than otherwise?


Also, I'm wondering how the monad works with "special" CEs like async. Does it for example have the same cancellation token functionality?

@gusty
Copy link
Member

gusty commented Nov 1, 2018

See

type StrictBuilder () =
inherit Builder ()
member inline __.Delay expr = expr : unit -> '``Monad<'T>``
member __.Run f = f () : '``Monad<'T>``
member __.TryWith (expr, handler) = try expr () with e -> handler e
member __.TryFinally (expr, compensation) = try expr () finally compensation ()
member rs.Using (disposable: #IDisposable, body) =
let body = fun () -> (body ()) disposable
rs.TryFinally (body, fun () -> dispose disposable)
type DelayedBuilder () =
inherit Builder ()
member inline __.Delay (expr: _->'``Monad<'T>``) = Delay.Invoke expr : '``Monad<'T>``
member __.Run f = f : '``Monad<'T>``
member inline __.TryWith (expr, handler ) = TryWith.Invoke expr handler : '``Monad<'T>``
member inline __.TryFinally (expr, compensation) = TryFinally.Invoke expr compensation : '``Monad<'T>``
member inline __.Using (disposable: #IDisposable, body) = Using.Invoke disposable body : '``Monad<'T>``

The main difference is that a lazy builder needs to handle special cases to deal with delayed computation.

As you can see in the code, it requires a custom Delay, TryWith, TryFinally and Using methods, whereas the strict one can use trivial definitions for those ones.

I'll have a look at how it relates to your implementation of the Result CE, but note that Result is in principle a strict CE, which explains why (together with your curiosity) you are finding so many issues here.

Your unit tests approach is great, but here it's more complicated, because we have to test the same but for many scenarios, it would be nice to add more tests here.

I'll have a look at your implementation.

Regarding the enumerators, that was my thought at the beginning, but this other issue you raised #137 makes me think that in fact all custom methods I mentioned above are somehow affected.

It behaves differently because as you can see the while implementation of a strict and a lazy one is different, and all these methods together should be aligned in the same evaluation strategy.

Regarding your question about async, it is my understanding that it should behave exactly the same, as the standard F# core async workflow is both lazy and fx (as opposed to monad plus), which are currently the defaults for simply monad.

@wallymathieu
Copy link
Member

wallymathieu commented Nov 1, 2018

What does the different names mean?

  • monad plus
  • monad fx

?

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 1, 2018

@gusty Thanks. I tried the strict monad', but that caused some compilation errors for my tests, which I now realize likely has to do with my result builder defining Zero as Ok ().

@gusty
Copy link
Member

gusty commented Nov 1, 2018

Yes, the different between an fx and a monadplus monad is that the combine function in the former is coded in such a way that allows to execute side-effects inside the CE, whereas the one in the monad-plus allows to combine many values (a monad-plus can return many times).

So for instance, a first return in an fx would be an early termination, but not in a monad-plus.

So, defining Zero as Ok () doesn't fit in either of these two models.

All this design was heavily inspired in the Computation Expressions Zoo paper.

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 1, 2018

Thanks. Do you know if it would be possible to define something akin to result and asyncResult (from Cvdm.ErrorHandling, see the readme for a very short explanation) using the generic CEs defined in FSharpPlus (possibly with slightly altered behavior compared to current result and asyncResult), or is this simply a case where separately defined CEs are needed?

@wallymathieu
Copy link
Member

What happens when you do:

  let getXResult someQ : Async<Result<XResult,KindOfError>> =
      monad {
         ...

?

@wallymathieu
Copy link
Member

wallymathieu commented Nov 1, 2018

You have also have similar constructs in ExtCore (as in your library). I did a trivial, but huge pull request. Some of the tests can be ported to f#+.

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 1, 2018

Doesn't work. If I try to constrain the types as much as possible, e.g.

let result : Result<string, string> =
      (monad { return x }: Async<Result<string, string>>)
      |> Async.RunSynchronously

then I get an error on x complaining about mismatch between string and Result<string, string>.

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 1, 2018

Thanks. I will experiment some more during the weekend.

@gusty
Copy link
Member

gusty commented Nov 1, 2018

In order to get a similar behavior to the asyncResult you need to use Monad Transformers.

The advantage of Monad Transformers is that you can combine the transformer with any monad, otherwise you would end up writing a lot of boilerplate, and any new monad will increase that boilerplate in an exponential way !

Monad transformers are also monads, but they have a different type, that's why your snippet doesn't work as you expect: the compiler is not expecting an asyncResult monad by the sole fact that the result contains a result inside the Async. If that was the case, it would be trying to be too smart and it will apply the asyncResult in cases whare just the async was intended.

You ommited the definition of x, but I guess it's a plain string, so you're failing code is probably something like:

let x = ""
let result : Result<string, string> =
      (monad { return x }: Async<Result<string, string>>)
      |> Async.RunSynchronously

You can get it working, of course by setting x to Ok "", but I think what you are after is this:

let x = ""
let result : Result<string, string> =
      (monad { return x }: ResultT<_>)
      |> ResultT.run
      |> Async.RunSynchronously

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 1, 2018

What is ResultT.run? I wasn't able to understand from the documentation/samples what monad transformers are or how they work.

@gusty
Copy link
Member

gusty commented Nov 1, 2018

Yes, there are no docs here that explain the concept from scratch, as there are very good Haskell tutorials in place, so the docs for now assume you are familiar with them.

Actually the same can be said about monads alone and so on.

It would be nice to have some explanation, but if we doc everything, from scratch, with good sample, we will end up creating an F# for fun and profit site on our docs.

Which is great ! But a lot of effort.

@gusty
Copy link
Member

gusty commented Nov 1, 2018

To answer your specific question:

ResultT.run is just un unwrapper for ResultT. So technically, a ResultT is a new type that wraps a Monad<Result<,>> (in your case the monad is Async).

This way the generic CE get the right overload which combines both monads (or more), once your CE stuff is done, you can get out of the wrapper with .run.

Not sure if it's clear, otherwise let me know.

@wallymathieu
Copy link
Member

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 2, 2018

Thanks, I'll have a look at the samples again with new eyes. In any case, I'd recommend linking from the documentation to those

very good Haskell tutorials

you mention.

@gusty
Copy link
Member

gusty commented Nov 2, 2018

Yes, we need at the very least provide links.
And ideally, translate the examples on the linked tutorial to F#, as I did with the Lens tutorial.

@gusty
Copy link
Member

gusty commented Nov 2, 2018

@cmeeren some stuff on monad transformers in F# in SO https://stackoverflow.com/questions/tagged/f%23+monad-transformers

@wallymathieu
Copy link
Member

Can we close this in favor of #140 ?

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 3, 2018

Sure!

@cmeeren cmeeren closed this as completed Nov 3, 2018
@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 7, 2018

@gusty In a previous comment, you said:

So for instance, a first return in an fx would be an early termination, but not in a monad-plus.

As I understand it, async in an fx monad. But according to your first comment in this thread, a first return is not an early termination in async workflows (since the side effect is executed). Could you clarify?

@gusty
Copy link
Member

gusty commented Nov 7, 2018

I'not sure I understand your question.

What I meant is that CEs not necessarily end at the return point, it all depends on how Combine is defined and a good example of this is a monad plus CE, like seq, where you can have multiple return (actually is called yield there, but it's the same mechanic).

@gusty
Copy link
Member

gusty commented Nov 7, 2018

I think it worth clarifying I'm not saying the opposite (actually here's the confusion) that all non monad plus CEs ends at the return point.

We can say that monadically talking they will end there, but they can have side effects as in that case.

Now using an if then else or an if then makes a difference, since the expression in an else won't be ever evaluated when the condition is true, but if it comes in the next line it will. Tricky I know.

@cmeeren
Copy link
Contributor Author

cmeeren commented Nov 7, 2018

What I meant is this:

You explicitly say in this comment that a first return in an fx monad is an early termination. I interpret this as meaning that no code after the first return will be executed.

However, in this comment, you say that in this code:

let threatLevel = 1
async {
  if threatLevel < 5 then
    return ()
  System.Console.WriteLine("Launch missiles")
} |> Async.RunSynchronously

then the side-effect will indeed be executed, even if it's after the first return.

Furthermore, my understanding is that async is an fx monad, and thus subject to the statement you made about early termination.

Thus, we have following incompatible statements:

  1. You: The first return in an fx monad is an early termination
  2. Me: Early termination means that no code will be executed after the termination (return)
  3. You: Side-effects can be executed after the first return in async, as the example shows
  4. Me: async is an fx monad

Could you clarify where the error is? I'm guessing it's point 2.

@gusty
Copy link
Member

gusty commented Nov 7, 2018

Sure, in short:

1-2 It's an early termination monadically speaking, I mean, no value will be returned afterwards, but side effects might still occur.

3-4 Async is definitely an fx monad

@gusty
Copy link
Member

gusty commented Oct 6, 2020

...

Also, there's other weird behavior. Consider this:

use enum = ([0] |> Seq.ofList).GetEnumerator()

let result = monad {
  while enum.MoveNext () do
    ignore enum.Current
}

This fails on enum.Current with System.InvalidOperationException : Enumeration has not started. Call MoveNext. But how is it possible to have arrived at the enum.Current line without MoveNext having been called?

@cmeeren FYI when #364 gets merged and released, it will make code like this to display a warning explaining what would happen at runtime and how to solve it.

Also, if you read the PR comment, you'll have a detailed description of why this happens.

So, after 2 years you should be on the safe side now ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants