The prompt rebinds the operators it says it rebinds (#1133) (#1134)

* The prompt rebinds the operators it says it rebinds

The Terminal opened `AggressiveOperators`, which rebinds `+ - * / **` *and* `= <> < > <= >=`. The
wiki says it rebinds five:

  "By default it applies aggressive operators which override the `+`, `-`, `*`, `/`, `**` operators
   for any pair of object. This is not an F# console, but AngouriMath terminal, after all."

The comparisons are not in that sentence and never were. What they cost is that `if`, `while`,
`List.filter` and a `match` guard all want a `bool` and were getting an `Entity`, so a large part of
ordinary F# did not typecheck at a prompt whose purpose is to be typed at. Measured, with and
without AngouriMath loaded on the same kernel: `if 1 < 2 then 1 else 0`, `let b: bool = 1 < 2`,
`[1;2;3] |> List.filter (fun x -> x > 1)` and an active pattern all failed on the comparison.

So the Terminal opens `ArithmeticOperators`, which is the five the documentation promises.
`AggressiveOperators` is untouched: it is public surface, its comparison overloads are deliberate and
tested, and somebody who wants them opens it at the prompt in one line. That was measured too --
`x > 0` is an ordinary comparison until that line and an inequality after it.

**What this does not fix, which is the honest half.** Rebinding arithmetic has the same shape of
cost, and it is the cost the tool exists to pay: once `+` and `*` build expressions, a function whose
branches mix an `Entity` with an `int` does not typecheck, and `List.sum` over expressions has no
`+` it can use. `let rec fib n = if n < 2 then n else fib (n-1) + fib (n-2)` still does not compile.
It fails on the arithmetic now rather than on the comparison, and #1133 used it as the headline
example, so it is pinned as a test with a remark saying so rather than left to be discovered as an
overclaim.

Narrowing bought back the part that was costing something and buying nothing.

Five of the seven cases in the issue compile now; the two that do not are the trade the tool makes.
41 terminal tests pass, and `AggressiveOperators`' own 18 are unchanged.

Part of #1133.

* fib compiles, once a session says which reading of the operators it wants

The previous commit narrowed what the prompt rebinds and left `fib` failing, on the grounds that
rebinding arithmetic is the trade the tool makes. That is true and it is not the whole answer: the
trade can be made per session rather than once, and then `fib` is available.

    open Microsoft.FSharp.Core.Operators              // fib 20 is 6765, and 3 / 2 is 1
    open AngouriMath.Interactive.ArithmeticOperators  // back: x + 1 is an expression, 3 / 2 is 3/2

Both directions measured, in one session, either following the other. `operators ()` prints them, so
it is discoverable from the prompt rather than from a wiki page.

**Why not by default.** Rebinding arithmetic and writing ordinary F# are mutually exclusive, which
was measured rather than reasoned: with F#'s own operators `fib 20` is 6765 and `3 / 2` is 1; with
arithmetic rebound `3 / 2` is 3/2 and `x + 1` is an expression, and `fib` does not typecheck. One
operator cannot have both return types.

An overload that keeps `int + int` an int was tried before concluding that -- a witness-constrained
SRTP operator over a base class carrying the general case. It resolves the simple cases and resolves
them all at once: `1 + 1` is 2, `x + 1` is an expression, `1 + 4.5` is 11/2. It does not resolve
`let rec`, because the operator has to be picked while the function's own type is still being
inferred, so it falls to the general case and `fib` fails anyway. Annotating the parameter and the
return type does not help either, which was checked rather than assumed. That is a negative result
about this mechanism, not about every mechanism.

The test that recorded `fib` as broken is replaced by one that runs it, and it fails if `fib` ever
starts compiling by default -- which would mean this reasoning had stopped being true and the remark
should be rewritten.

Part of #1133.

* Share a kernel across the terminal tests rather than starting one per case

Each case was calling createKernel, which starts an F# compiler service, and most of the new ones
then loaded AngouriMath into it as well: about thirty-four compiler services and sixteen full
AngouriMath loads for forty-one assertions.

Locally that was thirteen seconds and looked like nothing. On the macOS runner the
`Test Terminal Lib` step ran for over two hours without finishing, while the Linux and Windows legs
of the same run passed and the previous master run finished the same step in two minutes. So the
cost was real and this machine was hiding it.

xUnit runs the cases in a class one after another, so there was nothing to share a kernel with in
the first place. The cases that only ask the kernel to compile something now share one; the two
that change a session by `open`ing something still take a fresh one each, since that is the whole
of what they test.

Forty-one assertions, five seconds instead of thirteen.

* Run the terminal tests one at a time, because each starts a compiler service

Every test in this assembly starts an F# compiler service, and xUnit runs test classes in parallel
by default. Two modules were fine; the third made three kernels start together, and on the macOS
runner the `Test Terminal Lib` step then ran for over an hour against the two minutes it takes on
master, while the Linux and Windows legs of the same run passed.

Sharing a kernel within each class was the previous commit and was not enough on its own: it cut
thirty-four kernels to four, and the four still started at the same time. Measured rather than
assumed -- that commit went to CI and the step hung again.

Serialising is right on the merits rather than as a workaround. There is nothing to gain from
running two compiler services side by side on a three-core runner, and the assembly takes seven
seconds either way here.

The config is copied to the output directory, which is checked rather than assumed: a
xunit.runner.json that stays in the source tree configures nothing.

Author: Rafael-SOWNet

Sources/Terminal/AngouriMath.Terminal.Lib/PreRunCode.fs

@@ -19,7 +19,7 @@ let eval (x : obj) =
     | :? Entity.Number.Complex as cx -> cx.RealPart.EDecimal.ToString() + "" + "" + cx.ImaginaryPart.EDecimal.ToString() + ""i""
     | other -> (evaled other).ToString()
 
-open AngouriMath.Interactive.AggressiveOperators
+open AngouriMath.Interactive.ArithmeticOperators
 
 let x = symbol ""x""
 let y = symbol ""y""
@@ -39,6 +39,25 @@ let help () =
     psi.UseShellExecute <- true
     System.Diagnostics.Process.Start psi
     $""Sending you to {url}""
+
+/// Arithmetic here builds expressions, so `3 / 2` is a rational rather than 1 and `x + 1` is
+/// an expression rather than an error. The cost is that ordinary F# whose arithmetic must stay
+/// primitive does not typecheck: `let rec fib n = if n < 2 then n else fib (n-1) + fib (n-2)`
+/// is the usual example. These two say which reading you want, at any point in a session, and
+/// either can follow the other.
+///   open Microsoft.FSharp.Core.Operators           // ordinary F#: fib compiles, 3 / 2 is 1
+///   open AngouriMath.Interactive.ArithmeticOperators  // back to expressions
+/// https://github.com/asc-community/AngouriMath/issues/1133
+let operators () =
+    // Printed rather than returned: what a cell returns is handed to AngouriMath's formatter,
+    // which parses it, and a sentence is not an expression.
+    printfn ""Arithmetic builds expressions here, so 3 / 2 is 3/2 and x + 1 is an expression.""
+    printfn ""For ordinary F# - such as""
+    printfn ""    let rec fib n = if n < 2 then n else fib (n-1) + fib (n-2)""
+    printfn ""- type""
+    printfn ""    open Microsoft.FSharp.Core.Operators""
+    printfn ""and to come back to expressions,""
+    printfn ""    open AngouriMath.Interactive.ArithmeticOperators""
 "
 
 let enableAngouriMath kernel =

Sources/Tests/TerminalUnitTests/CompilerServiceWorksTest.fs

@@ -13,15 +13,24 @@ open AngouriMath.Terminal.Lib.Consts
 /// AngouriMath's own bindings. The Terminal opens AggressiveOperators over the top, which
 /// deliberately rebinds the comparison operators to build expressions — that is the tool's whole
 /// point and it is not what this asks about.
-let private bareKernel () =
+let private newKernel () =
     match createKernel () with
     | Result.Error _ ->
         Assert.True(false, "the kernel did not load at all")
         raise (System.Exception())
     | Result.Ok kernel -> kernel
 
+/// One kernel for the whole class, not one per case.
+///
+/// Creating a kernel starts a compiler service, which is the most expensive thing here by a wide
+/// margin, and xUnit runs the cases in a class one after another so there is nothing to share it
+/// with. Every case below only asks the kernel to compile something and reads the answer, so
+/// nothing one of them does changes what the next one sees — the cases that *do* change a session
+/// are in OperatorsAtThePromptTest and make their own.
+let private shared = lazy (newKernel ())
+
 let private evaluates (code: string) (expected: string) =
-    match execute (bareKernel ()) code with
+    match execute shared.Value code with
     | ExecutionResult.PlainTextSuccess actual -> Assert.Equal(expected, actual)
     | other ->
         Assert.True(false, $"expected {expected}, and the kernel answered {other}")
@@ -60,7 +69,7 @@ let ``The compiler service compiles F#`` (code: string) (expected: string) =
 [<InlineData("let x: int = \"not an int\"", "typecheck error")>]
 [<InlineData("let =", "parse error")>]
 let ``The compiler service reports errors`` (code: string) (expected: string) =
-    match execute (bareKernel ()) code with
+    match execute shared.Value code with
     | ExecutionResult.Error message ->
         Assert.Contains(expected, message)
         // A position, not just a complaint: the Terminal shows these to whoever typed them.
@@ -74,7 +83,7 @@ let ``The compiler service reports errors`` (code: string) (expected: string) =
 [<InlineData("derivative \"x\" \"x3 + sin(x)\"", "3 * x ^ 2 + cos(x)")>]
 [<InlineData("integral \"x\" \"x2\"", "x ^ 3 / 3 + C")>]
 let ``AngouriMath works through the compiler service`` (code: string) (expected: string) =
-    let kernel = bareKernel ()
+    let kernel = newKernel ()
     match enableAngouriMath kernel with
     | ExecutionResult.Error reason -> Assert.True(false, $"AngouriMath did not load: {reason}")
     | _ -> ()

Sources/Tests/TerminalUnitTests/OperatorsAtThePromptTest.fs

@@ -0,0 +1,136 @@
+/// Which operators the Terminal rebinds, and what that costs.
+/// https://github.com/asc-community/AngouriMath/issues/1133
+module AngouriMath.Terminal.OperatorsAtThePromptTest
+
+open Xunit
+open AngouriMath.Terminal.Lib.FSharpInteractive
+open AngouriMath.Terminal.Lib.PreRunCode
+open AngouriMath.Terminal.Lib.Consts
+
+let private newKernel () =
+    let k =
+        match createKernel () with
+        | Result.Error _ ->
+            Assert.True(false, "the kernel did not load")
+            raise (System.Exception())
+        | Result.Ok k -> k
+    match enableAngouriMath k with
+    | ExecutionResult.Error reason ->
+        Assert.True(false, $"AngouriMath did not load: {reason}")
+        raise (System.Exception())
+    | _ -> k
+
+/// One kernel for the cases that only read, for the reason CompilerServiceWorksTest gives: a
+/// kernel starts a compiler service and loads AngouriMath into it, and doing that once per case
+/// was thirteen full loads in this class alone. The cases that change a session — the two that
+/// `open` something — take a fresh one each, because that is the whole of what they are testing.
+let private shared = lazy (newKernel ())
+
+let private answers (k: Microsoft.DotNet.Interactive.FSharp.FSharpKernel) (code: string) (expected: string) =
+    match execute k code with
+    | ExecutionResult.PlainTextSuccess actual -> Assert.Equal(expected, actual)
+    | ExecutionResult.LatexSuccess (_, actual) -> Assert.Equal(expected, actual)
+    | other -> Assert.True(false, $"expected {expected}, and the prompt answered {other}")
+
+/// The Terminal rebinds the five arithmetic operators the wiki says it rebinds, and no longer
+/// rebinds the comparisons, which it never said it did.
+///
+/// The comparisons cost ordinary F#: `if`, `while`, `List.filter` and a `match` guard all want a
+/// `bool` and were getting an `Entity`, so a large part of the language did not typecheck at a
+/// prompt whose whole purpose is to be typed at.
+[<Theory>]
+[<InlineData("if 1 < 2 then 1 else 0", "1")>]
+[<InlineData("[1;2;3] |> List.filter (fun x -> x > 1)", "[2; 3]")>]
+[<InlineData("let b: bool = 1 < 2\nb", "True")>]
+[<InlineData("seq { 1..100 } |> Seq.filter (fun x -> x % 7 = 0) |> Seq.length", "14")>]
+[<InlineData("let (|Even|Odd|) n = if n % 2 = 0 then Even else Odd\nmatch 4 with Even -> \"e\" | Odd -> \"o\"", "e")>]
+let ``Ordinary F# with a comparison in it compiles`` (code: string) (expected: string) =
+    answers shared.Value code expected
+
+/// And the reading the Terminal exists for is untouched: arithmetic on anything still builds an
+/// expression, which is what the wiki documents and what makes this a symbolic prompt rather than
+/// an F# one.
+[<Theory>]
+[<InlineData("x + 1", "x + 1")>]
+[<InlineData("3 / 2", "3/2")>]
+[<InlineData("x ** 2", "x ^ 2")>]
+[<InlineData("solutions \"x\" \"x2 - 5x + 6 = 0\"", "{ 2, 3 }")>]
+[<InlineData("derivative \"x\" \"x3\"", "3 * x ^ 2")>]
+let ``The symbolic reading of arithmetic survives`` (code: string) (expected: string) =
+    answers shared.Value code expected
+
+/// What narrowing costs, stated rather than glossed: a bare `x > 0` no longer builds an
+/// inequality. It is one line to get back, and that line is the whole of the old behaviour.
+[<Fact>]
+let ``A comparison on symbols is one open away`` () =
+    let k = newKernel ()
+
+    match execute k "x > 0" with
+    | ExecutionResult.Error message -> Assert.Contains("comparison", message)
+    | other -> Assert.True(false, $"expected `x > 0` to be an ordinary comparison now, got {other}")
+
+    match execute k "open AngouriMath.Interactive.AggressiveOperators" with
+    | ExecutionResult.VoidSuccess -> ()
+    | other -> Assert.True(false, $"opening the aggressive operators answered {other}")
+
+    answers k "x > 0" "x > 0"
+
+/// <summary>
+/// <c>fib</c> compiles, which is what #1133 asked for — but not by default, and the difference is
+/// the point rather than a caveat.
+/// </summary>
+/// <remarks>
+/// <para>
+/// Rebinding arithmetic and writing ordinary F# are <b>mutually exclusive</b>, and that was
+/// measured rather than assumed. With F#'s own operators, <c>fib 20</c> is 6765 and <c>3 / 2</c> is
+/// <c>1</c>; with arithmetic rebound, <c>3 / 2</c> is <c>3/2</c> and <c>x + 1</c> is an expression
+/// while <c>fib</c> does not typecheck. Both readings of <c>/</c> cannot be the return type of one
+/// operator.
+/// </para>
+/// <para>
+/// An overload that keeps <c>int + int</c> an <c>int</c> was tried, as a witness-constrained SRTP
+/// operator, and it does resolve the simple cases — <c>1 + 1</c> is 2, <c>x + 1</c> is an
+/// expression, <c>1 + 4.5</c> is <c>11/2</c>, all at once. It does not resolve <c>let rec</c>: the
+/// operator has to be picked while the function's own type is still being inferred, so it falls to
+/// the general case and <c>fib</c> fails anyway. A type annotation does not help, which was checked
+/// rather than assumed.
+/// </para>
+/// <para>
+/// So the answer is that a session says which reading it wants. <c>operators ()</c> at the prompt
+/// prints the two lines, and either can follow the other any number of times.
+/// </para>
+/// </remarks>
+[<Fact>]
+let ``Ordinary F# is one open away, and reversible`` () =
+    let k = newKernel ()
+
+    // As the prompt starts: expressions, and fib does not compile.
+    answers k "x + 1" "x + 1"
+    answers k "3 / 2" "3/2"
+    match execute k "let rec fib n = if n < 2 then n else fib (n-1) + fib (n-2)\nfib 20" with
+    | ExecutionResult.Error _ -> ()
+    | other -> Assert.True(false, $"fib compiles by default now, so this test is out of date: {other}")
+
+    // One line, and it does.
+    match execute k "open Microsoft.FSharp.Core.Operators" with
+    | ExecutionResult.VoidSuccess -> ()
+    | other -> Assert.True(false, $"restoring F#'s operators answered {other}")
+    answers k "let rec fib n = if n < 2 then n else fib (n-1) + fib (n-2)\nfib 20" "6765"
+    answers k "[ for i in 1..10 -> i * i ] |> List.sum" "385"
+    answers k "3 / 2" "1"
+
+    // And back, in the same session.
+    match execute k "open AngouriMath.Interactive.ArithmeticOperators" with
+    | ExecutionResult.VoidSuccess -> ()
+    | other -> Assert.True(false, $"going back answered {other}")
+    answers k "x + 1" "x + 1"
+    answers k "3 / 2" "3/2"
+
+/// And the prompt says so, rather than leaving it to be discovered.
+[<Fact>]
+let ``The prompt explains which operators it has`` () =
+    // It prints rather than returns, so what is asserted is that it runs and says nothing back:
+    // a returned sentence would go to AngouriMath's formatter, which would try to parse it.
+    match execute shared.Value "operators ()" with
+    | ExecutionResult.VoidSuccess -> ()
+    | other -> Assert.True(false, $"operators () answered {other}")

Sources/Tests/TerminalUnitTests/TerminalUnitTests.fsproj

@@ -8,8 +8,19 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <!--
+    Every test in this assembly starts an F# compiler service, and xUnit runs test classes in
+    parallel by default. Three modules here means three kernels at once, which on the macOS runner
+    stopped finishing at all: the `Test Terminal Lib` step ran for over an hour against the two
+    minutes it takes on master, while the Linux and Windows legs of the same run passed. Sharing a
+    kernel within each class was not enough on its own, because the classes still start together.
+    https://github.com/asc-community/AngouriMath/issues/1133
+    -->
+    <None Include="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" />
+
     <Compile Include="Tests.fs" />
     <Compile Include="CompilerServiceWorksTest.fs" />
+    <Compile Include="OperatorsAtThePromptTest.fs" />
     <Compile Include="Program.fs" />
   </ItemGroup>
 

Sources/Tests/TerminalUnitTests/xunit.runner.json

@@ -0,0 +1,4 @@
+{
+  "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json",
+  "parallelizeTestCollections": false
+}

Sources/Wrappers/AngouriMath.Interactive/AngouriMath.Interactive.fsproj

@@ -29,6 +29,7 @@
     <None Include="Repack.ps1" />
 
     <Compile Include="KernelExtension.fs" />
+    <Compile Include="ArithmeticOperators.fs" />
     <Compile Include="AggressiveOperators.fs" />
   </ItemGroup>
 

Sources/Wrappers/AngouriMath.Interactive/ArithmeticOperators.fs

@@ -0,0 +1,29 @@
+/// The arithmetic operators, rebound to build expressions, and nothing else.
+///
+/// <c>AggressiveOperators</c> rebinds the comparisons as well, so that <c>x &gt; 0</c> is an
+/// inequality rather than a boolean. That is a real feature and it stays; what it costs is that
+/// ordinary F# containing a comparison stops typechecking, because <c>if</c>, <c>while</c>,
+/// <c>List.filter</c> and a <c>match</c> guard all want a <c>bool</c> where they now get an
+/// <c>Entity</c>. <c>let rec fib n = if n &lt; 2 then n else fib (n-1) + fib (n-2)</c> is the first
+/// recursive function most people type at an F# prompt, and it does not compile.
+///
+/// This is the half a prompt wants by default: arithmetic on anything builds an expression, and
+/// the language keeps working. Somebody who wants the comparisons too opens
+/// <c>AngouriMath.Interactive.AggressiveOperators</c> over the top of it, which is one line and
+/// restores exactly the old behaviour.
+///
+/// https://github.com/asc-community/AngouriMath/issues/1133
+module AngouriMath.Interactive.ArithmeticOperators
+
+open AngouriMath.FSharp.Core
+
+let ( + ) a b =
+    ((parsed a) + (parsed b)).InnerSimplified
+let ( - ) a b =
+    ((parsed a) - (parsed b)).InnerSimplified
+let ( * ) a b =
+    ((parsed a) * (parsed b)).InnerSimplified
+let ( / ) a b =
+    ((parsed a) / (parsed b)).InnerSimplified
+let ( ** ) a b =
+    ((parsed a).Pow(parsed b)).InnerSimplified