From 1ba039866fc8d5aeb9df5700120d371271cb0f0e Mon Sep 17 00:00:00 2001 From: Casper Bollen Date: Sun, 29 Oct 2023 11:01:38 +0100 Subject: [PATCH] fix: not working perf improvement using memoization and broken test --- ...rogram.EquationBenchmarks-report-github.md | 26 ++--- src/Informedica.GenSolver.Lib/Equation.fs | 98 ++++++++++--------- .../Scripts/Solver.fsx | 2 + src/Informedica.GenSolver.Lib/Solver.fs | 32 +----- tests/Informedica.GenSolver.Tests/Tests.fs | 2 +- 5 files changed, 70 insertions(+), 90 deletions(-) diff --git a/benchmark/BenchmarkDotNet.Artifacts/results/Program.EquationBenchmarks-report-github.md b/benchmark/BenchmarkDotNet.Artifacts/results/Program.EquationBenchmarks-report-github.md index d0ffd57..6eaea4f 100644 --- a/benchmark/BenchmarkDotNet.Artifacts/results/Program.EquationBenchmarks-report-github.md +++ b/benchmark/BenchmarkDotNet.Artifacts/results/Program.EquationBenchmarks-report-github.md @@ -8,16 +8,16 @@ Apple M2 Max, 1 CPU, 12 logical and 12 physical cores ``` -| Method | Mean | Error | StdDev | -|-------------------- |------------:|------------:|------------:| -| AllPairesInt_100 | 124.4 μs | 0.50 μs | 0.46 μs | -| AllPairsInt_200 | 740.2 μs | 4.20 μs | 3.93 μs | -| SolveCountMinIncl | 100.3 μs | 0.56 μs | 0.49 μs | -| Solve_1_Eqs_100 | 12,808.2 μs | 123.12 μs | 115.17 μs | -| Solve_1_Eqs_200 | 51,027.0 μs | 1,018.90 μs | 2,034.85 μs | -| Solve_3_Eqs_100 | 13,494.7 μs | 140.81 μs | 131.72 μs | -| Solve_3_Eqs_200 | 52,360.1 μs | 997.85 μs | 1,109.10 μs | -| Solve_1_Eqs_Rand_10 | 9,078.6 μs | 23.49 μs | 20.82 μs | -| Solve_1_Eqs_Rand_20 | 89,190.0 μs | 441.00 μs | 412.51 μs | -| Solve_3_Eqs_Rand_10 | 9,599.2 μs | 28.00 μs | 26.19 μs | -| Solve_3_Eqs_Rand_20 | 90,438.7 μs | 358.47 μs | 335.31 μs | +| Method | Mean | Error | StdDev | +|-------------------- |-------------:|-------------:|-------------:| +| AllPairesInt_100 | 124.16 μs | 0.235 μs | 0.208 μs | +| AllPairsInt_200 | 750.41 μs | 3.810 μs | 3.377 μs | +| SolveCountMinIncl | 97.92 μs | 1.198 μs | 1.120 μs | +| Solve_1_Eqs_100 | 12,788.84 μs | 134.926 μs | 126.210 μs | +| Solve_1_Eqs_200 | 50,817.55 μs | 1,013.622 μs | 2,093.306 μs | +| Solve_3_Eqs_100 | 13,385.99 μs | 81.075 μs | 75.838 μs | +| Solve_3_Eqs_200 | 51,935.72 μs | 993.614 μs | 1,182.827 μs | +| Solve_1_Eqs_Rand_10 | 9,183.25 μs | 27.803 μs | 26.007 μs | +| Solve_1_Eqs_Rand_20 | 88,937.43 μs | 55.626 μs | 46.450 μs | +| Solve_3_Eqs_Rand_10 | 9,415.45 μs | 19.153 μs | 16.978 μs | +| Solve_3_Eqs_Rand_20 | 94,196.45 μs | 260.758 μs | 231.155 μs | diff --git a/src/Informedica.GenSolver.Lib/Equation.fs b/src/Informedica.GenSolver.Lib/Equation.fs index be7f170..b7a3e22 100644 --- a/src/Informedica.GenSolver.Lib/Equation.fs +++ b/src/Informedica.GenSolver.Lib/Equation.fs @@ -336,6 +336,52 @@ module Equation = | y::xs -> y |> op2 <| (xs |> List.reduce op1) |> Some + // select the right application operator + let (<==) = if onlyMinIncrMax then (@<-) else (^<-) + // perform the calculations on the vars + let calcVars op1 op2 vars = + vars + |> List.fold (fun acc vars -> + if acc |> Option.isSome then acc + else + match vars with + | _, [] + | _, [ _ ] -> acc + | i, y::xs -> + let op2 = if i = 0 then op1 else op2 + // log starting the calculation + (op1, op2, y, xs) + |> Events.EquationStartCalculation + |> Logging.logInfo log + + xs + |> calc op1 op2 + |> function + | None -> + // log finishing the calculation + (y::xs, false) + |> Events.EquationFinishedCalculation + |> Logging.logInfo log + + None + | Some var -> + let yNew = y <== var + + if yNew <> y then + // log finishing the calculation + ([yNew], true) + |> Events.EquationFinishedCalculation + |> Logging.logInfo log + + Some yNew + else + // log finishing the calculation + ([], false) + |> Events.EquationFinishedCalculation + |> Logging.logInfo log + + None + ) None if eq |> isSolved then eq, Unchanged else @@ -343,8 +389,7 @@ module Equation = eq |> Events.EquationStartedSolving |> Logging.logInfo log - // select the right application operator - let (<==) = if onlyMinIncrMax then (@<-) else (^<-) + // get the vars and the matching operators let vars, op1, op2 = match eq with @@ -362,50 +407,6 @@ module Equation = // a = b + d becomes a list representing // [ a = b + d; b = a - d; d = a - b ] let vars = vars |> reorder - // perform the calculations on the vars - let calcVars vars = - vars - |> List.fold (fun acc vars -> - if acc |> Option.isSome then acc - else - match vars with - | _, [] - | _, [ _ ] -> acc - | i, y::xs -> - let op2 = if i = 0 then op1 else op2 - // log starting the calculation - (op1, op2, y, xs) - |> Events.EquationStartCalculation - |> Logging.logInfo log - - xs - |> calc op1 op2 - |> function - | None -> - // log finishing the calculation - (y::xs, false) - |> Events.EquationFinishedCalculation - |> Logging.logInfo log - - None - | Some var -> - let yNew = y <== var - - if yNew <> y then - // log finishing the calculation - ([yNew], true) - |> Events.EquationFinishedCalculation - |> Logging.logInfo log - - Some yNew - else - // log finishing the calculation - ([], false) - |> Events.EquationFinishedCalculation - |> Logging.logInfo log - - None - ) None // loop until no changes are detected, i.e. // calcVars returns None let rec loop acc vars = @@ -419,7 +420,7 @@ module Equation = |> List.sumBy Variable.count ) - match calcVars vars with + match calcVars op1 op2 vars with | None -> acc, vars | Some var -> vars @@ -434,7 +435,8 @@ module Equation = |> List.sortBy(fun (_, xs) -> xs |> List.tail - |> List.sumBy Variable.count + |> List.map Variable.count + |> List.reduce (*) ) |> loop (acc |> List.replaceOrAdd (Variable.eqName var) var) diff --git a/src/Informedica.GenSolver.Lib/Scripts/Solver.fsx b/src/Informedica.GenSolver.Lib/Scripts/Solver.fsx index a23a18f..bd67bc5 100644 --- a/src/Informedica.GenSolver.Lib/Scripts/Solver.fsx +++ b/src/Informedica.GenSolver.Lib/Scripts/Solver.fsx @@ -137,3 +137,5 @@ eqs |> ignore | Error _ -> failwith "errors" |> ignore + + diff --git a/src/Informedica.GenSolver.Lib/Solver.fs b/src/Informedica.GenSolver.Lib/Solver.fs index fe5af51..81b37d6 100644 --- a/src/Informedica.GenSolver.Lib/Solver.fs +++ b/src/Informedica.GenSolver.Lib/Solver.fs @@ -6,8 +6,6 @@ namespace Informedica.GenSolver.Lib /// equations module Solver = - open System.Collections.Generic - module EQD = Equation.Dto module Name = Variable.Name @@ -80,25 +78,6 @@ module Solver = , rst - /// - /// Create a memoized function - /// - /// The function to memoize - /// - /// This memoize function uses a Dictionary instead of - /// a map, because the Dictionary is faster. But Dictionary - /// cannot take Unit as a key! - /// - let memSolve f = - let cache = Dictionary<_, _>() - fun e -> - if cache.ContainsKey(e) then cache[e] - else - let r = f e - cache.Add(e, r) - r - - let sortQue onlyMinMax que = if que |> List.length = 0 then que else @@ -111,6 +90,7 @@ module Solver = /// and function to determine whether an /// equation is solved let solve onlyMinIncrMax log sortQue var eqs = + let solveE n eqs eq = try Equation.solve onlyMinIncrMax log eq @@ -135,7 +115,7 @@ module Solver = |> Exceptions.SolverTooManyLoops |> Exceptions.raiseExc None [] - let que = que |> sortQue + let que = que |> sortQue onlyMinIncrMax //(n, que) //|> Events.SolverLoopedQue @@ -245,10 +225,8 @@ module Solver = //TODO: need to clean up the number check let solveVariable onlyMinIncrMax log sortQue vr eqs = let n1 = eqs |> List.length - // TODO: need to test this using BenchmarkDotNet let solve = - solve onlyMinIncrMax log (sortQue onlyMinIncrMax) None - |> memSolve + solve onlyMinIncrMax log sortQue (Some vr) match solve eqs with | Error (eqs, errs) -> Error (eqs, errs) @@ -261,10 +239,8 @@ module Solver = //TODO: need to clean up the number check let solveAll onlyMinIncrMax log eqs = let n1 = eqs |> List.length - // TODO: need to test this using BenchmarkDotNet let solve = - solve onlyMinIncrMax log (sortQue onlyMinIncrMax) None - |> memSolve + solve onlyMinIncrMax log sortQue None match solve eqs with | Error (eqs, errs) -> Error (eqs, errs) diff --git a/tests/Informedica.GenSolver.Tests/Tests.fs b/tests/Informedica.GenSolver.Tests/Tests.fs index 71b7750..1ebc250 100644 --- a/tests/Informedica.GenSolver.Tests/Tests.fs +++ b/tests/Informedica.GenSolver.Tests/Tests.fs @@ -166,7 +166,7 @@ module TestSolver = let solve n p eqs = let n = n |> Name.createExc - Api.solve true id logger n p eqs + Api.solve true (fun _ eqs -> eqs) logger n p eqs let solveAll = Api.solveAll false logger