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

[RFC FS-1063] Applicative CEs via and! and BindReturn #7756

Merged
merged 9 commits into from
Jan 28, 2020

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Oct 22, 2019

Implementation of F# RFC FS-1063 and! and BindReturn for more efficient computation expressions

Continues #5696

TODO:

  • Put the interaction of these new features with custom operators under test

  • check langversion checks being applied correctly

  • add tests for langversion not set

@dsyme
Copy link
Contributor Author

dsyme commented Oct 22, 2019

I’m going to put some work in to try to get this into F# 5.0, though it's by no means certain.

I’m particularly interested in the use case for adaptive data in https://github.com/fsprojects/FSharp.Data.Adaptive/ but I need to check that the tight restrictions on applicatives aren’t going to be a killer problem. So I’d just like to quiz you on your design choices and understand if they are going to be a problem

@TD5 thanks for writing such an incredibly complete RFC, it’s really impressive.

Specifically, FSharp.Data.Adaptive defines builders

           adaptive { … }
           aset { … }
           alist { … }
           amap { … }

all of which at a high level feel like they should be able to support let-bang/and-bang (I think…). However I’m concerned that the restriction to monads and not monoids will rule out the last three.

I know there’s an extensive discussion of this in the RFC, I will try to understand it better. I’ll also try to understand the implementation a little better 😊

Help appreciated in proofing this for those who have the inclination

@TD5
Copy link
Contributor

TD5 commented Oct 22, 2019

@dsyme Some of the restrictions are in place just because I was trying to keep the scope fairly tight, and so there's no technical reason that they couldn't be reworked or extended in later RFCs to admit more interesting computations. FWIW, I did have incremental computations in mind when I was designing this at the time, although a year on it's a bit hazy 😅

Things are a busy for me at the moment, but I am in principle happy to discuss the details with you.

However I’m concerned that the restriction to monads and not monoids will rule out the last three.

I am not sure if you're referring to a restriction from the let! ... and! ... RFC here or FSharp.Data.Adaptive. It certainly is fair to say I avoided doing anything smart with monoids in the proposal (see the section Using yield where I basically reject anything but the most simple usage), but I am not sure where monads come into it - none of the examples I give require you to define Bind in order to use yield, IIRC.

Conceptually, the relationship between monoids and applicatives has been explored before (e.g. in Haskell's Alternatives and related types and discussions), so I am confident we can make any tweaks necessary to get the features you want. In fact, I am keen to see that aspect of the proposal refined a bit more if there is a general consensus that it wouldn't make the overall change too big.

I think I'll need to review FSharp.Data.Adaptive in order to say anything more concrete at this stage.

@dsyme
Copy link
Contributor Author

dsyme commented Oct 22, 2019

@TD5 Thanks so much for replying quickly, given how long it's taken me to look at this properly

I am not sure where monads come into it

Yes, I meant traditional applicatives, not monads

@dsyme
Copy link
Contributor Author

dsyme commented Oct 22, 2019

@TD5 I'm still getting a hang of this, so let me ask some things about the design.

The Apply translation pushing down into Return is at the core of the design and feels quite complex and likely to cause a lot of restrictions.

Did you consider a design that's based on an overloaded n-ary Bind, e.g.

        member x.Bind(value: aval<'T1>, mapping: 'T1 -> aval<'T2>) =
            AVal.bind mapping value

        member x.Bind2(value1: aval<'T1>, value2: aval<'T2>, mapping: 'T1 -> 'T2 -> aval<'T3>) =
            AVal.bind2 mapping value1 value2

There are three obvious downsides to this, notably

  1. placing an upper limit on number of and!
  2. interacting badly with anduse!
  3. may require a quadratic number of overloads of Bind is already overloaded

Putting these aside, I'm wondering if there are advantages in that the translation becomes more local and the interactions with other features becomes clearer?

It's just that when I look purely syntactically at

    ce { 
        let! x = ...
        and! y = ...
        ...
    }

and wonder to myself "what would this translate to given the rest of the F# design", I wouldn't be totally surprised if the answer was "it becomes a call to builder.Bind2"....

If the Map RFC is implemented, then it would become builder.Map2 if the result is a bind-free computation (i.e. all of whose branches are a return). So

        member x.Map(value: aval<'T1>, mapping: 'T1 -> 'T2) =
            AVal.map mapping value

        member x.Map2(value1: aval<'T1>, value2: aval<'T2>, mapping: 'T1 -> 'T2 -> 'T3) =
            AVal.map2 mapping value1 value2

etc.

I'm not sure what I'd expect uses of anduse! ... to become, I want to shelve that for a moment though.

Note this is looking from a syntactic point of view (and computation expressions are largely a syntactic device).

@dsyme
Copy link
Contributor Author

dsyme commented Oct 22, 2019

A sketch of what it would be to add Bind2 to FSharp.Data.Adaptive is here: https://github.com/fsprojects/FSharp.Data.Adaptive/compare/master...dsyme:bind2?expand=1#diff-ac5f27d14fb5acd508090742e53062e7R12

An addition of just Apply to AValBuilder builder (adaptive { ... }) would be even simpler. But this diff shows that it's also realistic to add Bind2 to AListBuilder (alist { ... }) and friends.

@TD5
Copy link
Contributor

TD5 commented Oct 22, 2019

Did you consider a design that's based on an overloaded Bind and Map?

I never considered overloading utilising overloading. That's presumably just because my personal F# style avoids overloading, so that'd be a valid and interesting design space to explore.

Aside from overloading per se, I think there are two other points raised here: Pulling the Map/functor case into scope for this change, and not actually supporting apply, as such, but using Map, Map2, ..., MapN and Bind rather than an explicit Apply.

Pulling Map into scope for this change:

I am definitely open to that. At one point I think I was keen on this, but we discussed it and decided to leave that as an extension/separate proposal to keep this one simple(r) and to defer a decision on whether to invoke more efficient rewrites (e.g. avoiding Binds where the user's code immediately returns - see the RFC on this topic). As you say, our hand may be forced depending on whatever other design changes we decide to make here.

Using Map, Map2, ... and Bind without an explicit Apply:

I remember going down the route of defining the applicative CEs in terms of Apply because:

  • Apply is the canonical form generally seen in the research regarding applicatives, so I was most familiar with that and found it most natural
  • As you say, Apply scales to as many arguments as you want quite conveniently, and it does so more uniformily (I'd hate to have a bug where Map and Map2 disagree in some odd way).

i.e. I think the choice was fairly subjective.

Looking at the RFC now, I find it interesting that all of the examples utilise Return, and the syntax doesn't admit the situation where we have a wrapped function (e.g. aval<'T1 -> 'T2>) to be applied using Apply. That seems like a mistake to me since it doesn't take full advantage of the opportunity to make using applicatives convenient (although you can of source still use let! ... and! ... to bind your wrapped function and then make use of it inside return). This something that could and probably should be admitted, although I haven't particularly thought about the syntax for that case yet.

I suppose one other consideration is how often we expect the function to be already wrapped vs. not wrapped (i.e. the f in the canonical form f <*> x <*> yor pure f <*> x <*> y, respectively) - presumably having the CE implementation support either wrapped (Apply), or not wrapped (MapN), functions gives a slight performance trade-off in that situation.


As an aside, when considering support for monoids, the current formulation for let! ... and! ... requiring return to be defined sort of moves us towards more explicit support for monoids/alternatives, in that I think any reasonable implementation would probably have that be the identity for <|> (or whatever you call the monoidal append/alternation operator).

@dsyme
Copy link
Contributor Author

dsyme commented Oct 22, 2019

@TD5 Cool, thanks for those notes.

Another design option: instead of Bind2/... we tuple up the bind sources pairwise and untuple them in the bind continuation. So to add support for let! ... and! ... a builder would have:

member x.CombineSources(value1: aval<'T1>, value2: aval<'T2>) : aval<'T1 * 'T2>

and then

ce { let! v1 = e1 and! v2 = e2 in ... }

becomes

ce.Bind(ce.CombineSources(e1, e2), (fun (v1, v2) -> ... )

with automatic nesting for CombineSources. This would give fewer overloads - just add CombineSources and you're done. And the feature would be orthogonal to Map.

@krauthaufen
Copy link
Contributor

hey @dsyme the CombineSources combinator would also work structurally, which means you could e.g. define it to be left-associative or something similar:

ce.Bind(CombineSources(CombineSources(a,b),c), fun (a,(b,c)) -> ...)

which would only require a minimal change to the way things currently are and allow for arbitrarily long and! chains

@krauthaufen
Copy link
Contributor

with automatic nesting for CombineSources. This would give fewer overloads - just add CombineSources and you're done. And the feature would be orthogonal to Map.

just saw that 😳

@krauthaufen
Copy link
Contributor

Btw. I came up with a sketch allowing to use yield / etc. inside applicative binds, that is very close to Haskell's way of encoding applicatives, but I think @dsyme's proposal is actually way easier to understand and also a way smaller change to the F# language.

Nonetheless if someone's interested: https://gist.github.com/krauthaufen/afa861e988ca1a21570febf6d02a214c

@TD5
Copy link
Contributor

TD5 commented Oct 22, 2019

Regarding ce.Bind2(ce.CombineSources(e1, e2), (fun (v1, v2) -> ... ), with:

    member x.Bind2(value1: aval<'T1>, value2: aval<'T2>, mapping: 'T1 -> 'T2 -> aval<'T3>) =
        AVal.bind2 mapping value1 value2

That seems to assume Bind can or should be defined at all for your computation. Some computations don't allow bind simply because it does not make sense, or bind is prohibitively expensive or considered dangerous or not idiomatic. N.B. this does not apply to the Map & Select, or Map, Map2, ..., MapN suggestions, which are both equal in power to the Pure & Apply formulation.

Part of the motivation, in my mind, for let! ... and! ... is that we can get syntactic sugar for things that make sense as applicatives but not monads. There are lots of examples of this floating around, e.g. ZipLists don't really work as monads, but can be quite natural as applicatives, or in the formlets examples from the RFC, using bind would stop us being able to statically inspect the full shape of the computation since we can't "see inside" the function argument to Bind, and that sort of defeats the point of the computation.

Additionally (and this is really just a presumption on my part), some operations may be more efficient in terms of apply rather than bind, because we know we don't need to go through the step of generating the continuation of the computation. Then there's an argument to be had around the value of an and! in the source guaranteeing that Bind would not be called if the applicative formulation was available.

So going back to ce.Map2(ce.CombineSources(e1, e2), (fun (v1, v2) -> ... ), yes, that seems equivalent and definitiely has potential as long as there's an option to not have Bind, Bind2, ... defined at all.

@krauthaufen
Copy link
Contributor

So going back to, ce.Map2(ce.CombineSources(e1, e2), (fun (v1, v2) -> ... ), yes, that seems equivalent and definitiely has potential as long as there's an option to not have Bind, Bind2, ... defined at all.

that would simply be using Map (without the 2) wouldn't it?
If i understood that correctly the CombineSources approach would no longer require Bind2 or Map2...

I think it would nicely separate multi-binds from dealing with non-monadic types.
From our (FSharp.Data.Adaptive) perspective the multi-bind aspect would be super-cool, but dropping monad-support when using multi-binds is a deal-breaker for adaptive.

So non-monadic builders would certainly need the proposed Map functionality

@TD5
Copy link
Contributor

TD5 commented Oct 22, 2019

that would simply be using Map (without the 2) wouldn't it?
If i understood that correctly the CombineSources approach would no longer require Bind2 or Map2...

Ah yes, good spot! That's what I knew as Merge and Select, and I think it's a perfectly valid approach.

@krauthaufen
Copy link
Contributor

krauthaufen commented Oct 22, 2019

Hey, applicatives could (even without the Map proposal) be implemented with the multi-binds by simply "lying" in the types forBind and Return. That way only one large multi-bind, followed by a simple return is allowed in the CE.

type Applicative() =
    member x.Return v = v

    member x.CombineSources(a : option<'a>, b : option<'b>) =
        (a,b) ||> Option.map2 (fun a b -> (a,b))

    member x.Bind(m : option<'a>, mapping : 'a -> 'b) = 
        m |> Option.map mapping

let test (a : option<int>) (b : option<int>) (c : option<int>) =
    let app = Applicative()
    app {
        // let! a = a
        // and! b = b
        // and! c = c
        let! ((a, b), c) = app.CombineSources(app.CombineSources(a, b), c)
        let x = c * c + a
        return a * b + x
    }

@dsyme
Copy link
Contributor Author

dsyme commented Oct 23, 2019

Ah yes, good spot! That's what I knew as Merge and Select, and I think it's a perfectly valid approach.

MergeSources may be a better name.

Are there CEs where it makes sense to have MergeSources, Map and some of the other constructs such as TryFinally or TryWith?

@krauthaufen
Copy link
Contributor

Are there CEs where it makes sense to have MergeSources, Map and some of the other constructs such as TryFinally or TryWith?

I am thinking about async with parallel dependencies...
When implemented correctly computeA and computeB could potentially run in parallel...

async {
    try
        let! a = computeA
        and! b = computeB
        return a + b
    finally
        printfn "done"
}

@dsyme
Copy link
Contributor Author

dsyme commented Oct 23, 2019

I've pushed a trial implementation where everything is simplified to just an overloaded MergeSources

Some tests have been added and the others adjusted.

Caveats

  • This doesn't yet tuple/untuple automagically so overloads of MergeSources are required for 3, 4 etc. simultaneous let! ... and! ... . I suppose we should add the automatic tupling/untupling.

  • This doesn't meet the RFC because anduse! is not supported. But I suspect we can live with that. @TD5 how strong do you think the use cases are for this?

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a parseState.LexBuffer.SupportsFeature inside the pars.fsy file, just like our other features that added/changed syntax in 4.7

src/fsharp/service/ServiceStructure.fs Show resolved Hide resolved
src/fsharp/service/ServiceUntypedParse.fs Show resolved Hide resolved
@dsyme
Copy link
Contributor Author

dsyme commented Oct 23, 2019

Failures:

CompletionInDifferentEnvs3
CompletionInDifferentEnvs5

due to some difference in error recovery in parsing bindings due to changes in parser

@TD5
Copy link
Contributor

TD5 commented Oct 24, 2019

@dsyme I don't have a particular use of anduse! in mind, so I wouldn't say I have a strong use case for it. I think there's some merit to the idea that F# users might expect it would exist for applicative CEs if it exists for monadic ones, but that's it.

@dsyme
Copy link
Contributor Author

dsyme commented Oct 24, 2019

The feature Map for computation expressions has been incorporated into this RFC/PR - though Map is now called BindReturn.

A quick summary of the relevant new builder methods:

  • MergeSources - gives and! support for arbitrary number of and! via tupling nodes

  • MergeSources3, MergeSources4... - optional, reduce number of tupling nodes

  • Bind2, Bind3 etc. - optional, binds let! .. and! ... efficiently without tupling nodes

  • BindReturn - adds support and/or efficiency for let! ... return

  • Bind2Return, Bind3Return, etc. - optional, binds let! .. and! ... return efficiently without tupling nodes

@dsyme
Copy link
Contributor Author

dsyme commented Oct 24, 2019

The dependency graph sample is here: https://github.com/dotnet/fsharp/pull/7756/files#diff-e77ad28a19cb44b6c0d896f17fac41f6

This gives a measure of the way and! and BindReturn can give massive performance differences, from O(n) or O(n^2) or whatever to O(1)

//total recalcs using and! = 2000
//total nodes using and! = 1
//----
//total recalcs using let! = 6000
//total nodes using let! = 4003

@dsyme dsyme changed the title [WIP, RFC FS-1063] Add let! ... and! ... syntax #5696 [WIP, RFC FS-1063] More efficient and expressive computation expressions via and! and BindReturn Oct 24, 2019
@dsyme dsyme changed the title [WIP, RFC FS-1063] More efficient and expressive computation expressions via and! and BindReturn [WIP, RFC FS-1063] More efficient and expressive CEs via and! and BindReturn Oct 24, 2019
@dsyme dsyme changed the title [WIP, RFC FS-1063] More efficient and expressive CEs via and! and BindReturn [WIP, RFC FS-1063] Applicative CEs via and! and BindReturn Oct 24, 2019
@krauthaufen
Copy link
Contributor

Hey @dsyme I've started implementing the new builder methods in FSharp.Data.Adaptive and I stumbled upon two questions:

  • are the functions given to Bind2/Bind2Return curried or tupled? Bind2Return : m<'a> * m<'b> * ('a -> 'b -> 'c) vs. Bind2Return : m<'a> * m<'b> * ('a * 'b -> 'c)
  • does MergeSources3 have type m<'a> * m<'b> * m<'c> -> m<'a * ('b * 'c)> or m<'a> * m<'b> * m<'c> -> m<'a * 'b * 'c>?

@krauthaufen
Copy link
Contributor

@dsyme did you consider also adding ForYield which would basically have the same signature as BindReturn?

The idea would be to use map instead of collect in scenarios like:

aset {
    for a in set do
        yield 2 * a
}

so the signature would need to be something like ForYield : m<'a> -> ('a -> 'b) -> m<'b>

krauthaufen added a commit to fsprojects/FSharp.Data.Adaptive that referenced this pull request Oct 25, 2019
* implemented new builder methods as proposed in dotnet/fsharp#7756
* implemented `AVal.bind3`
@dsyme
Copy link
Contributor Author

dsyme commented Oct 25, 2019

did you consider also adding ForYield which would basically have the same signature as BindReturn?

I did think about this on my commute home and it's good to see you think of it too, and that it is relevant for FSharp.Data.Adaptive

I suppose there is another feature lurking here for x and y in xs and ys do ... for iterating two collections in parallel, akin to let! ... and! ... though I don't think we'll go there

@dsyme
Copy link
Contributor Author

dsyme commented Oct 25, 2019

I did think about this on my commute home and it's good to see you think of it too, and that it is relevant for FSharp.Data.Adaptive

(note I'm not entirely sure it's worth adding - would people understand the requirements for successful use? And for collections people seem more likely to happily use ASet.map etc. )

@krauthaufen
Copy link
Contributor

Thanks, I'll adjust the implementations in FDA eventually...

@krauthaufen
Copy link
Contributor

I personally prefer the combinator approach in most cases but there are some where it can get a little tedious like:

AList.append
    (AList.single a)
    (AList.append (AList.map foo bar) (AList.single b))

Nonetheless I don't think the ForYield implementation is super important...

Regarding the for ... and thing I'm not even sure what is the proper semantic for it (all combinations vs. zip), so I definetly wouldn't go there 😁

Cheers

@TD5
Copy link
Contributor

TD5 commented Oct 25, 2019

Regarding the for ... and thing I'm not even sure what is the proper semantic for it (all combinations vs. zip), so I definetly wouldn't go there 😁

Zip's a great example of something that can be expressed as an applicative but not a monad 😁

krauthaufen added a commit to fsprojects/FSharp.Data.Adaptive that referenced this pull request Oct 28, 2019
@dsyme dsyme changed the title [WIP, RFC FS-1063] Applicative CEs via and! and BindReturn [RFC FS-1063] Applicative CEs via and! and BindReturn Oct 28, 2019
@dsyme
Copy link
Contributor Author

dsyme commented Oct 28, 2019

This is basically done. I've removed the WIP tag for now to indicate it's available for review and usage testing and likely other criteria we need to pass

@KevinRansom
Copy link
Member

@dsyme , can we target this to fsharp5, please.

@dsyme dsyme changed the base branch from master to release/fsharp5 November 20, 2019 18:28
@dsyme
Copy link
Contributor Author

dsyme commented Jan 10, 2020

@cartermp I've resolved all the outstanding issues with the testing and fixed an issue in the interaction with custom operators (discovered during testing)

I'll now revise the RFC, but this feature is basically now ready for "preview" status when reviewed etc.

@dsyme dsyme changed the base branch from release/fsharp5 to master January 16, 2020 11:34
@KevinRansom KevinRansom merged commit 3f9172a into master Jan 28, 2020
@cartermp
Copy link
Contributor

should the feature branch get deleted?

nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
Co-authored-by: Kevin Ransom (msft) <codecutter.fsharp@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants