From 385d10532f972062cf6454eecb6bd1e6c4d0d9ec Mon Sep 17 00:00:00 2001 From: Casper Bollen Date: Sun, 29 Oct 2023 15:21:28 +0100 Subject: [PATCH] refact: removed unnecessary onlyMinIncrMax and added comments --- src/Informedica.GenOrder.Lib/OrderVariable.fs | 2 +- src/Informedica.GenSolver.Lib/Api.fs | 66 ++++++++++++++----- src/Informedica.GenSolver.Lib/Constraint.fs | 39 ++++++++++- src/Informedica.GenSolver.Lib/Equation.fs | 4 +- src/Informedica.GenSolver.Lib/Solver.fs | 42 +++++++++--- src/Informedica.GenSolver.Lib/Types.fs | 4 +- src/Informedica.GenSolver.Lib/Variable.fs | 22 ++----- tests/Informedica.GenSolver.Tests/Tests.fs | 2 +- 8 files changed, 131 insertions(+), 50 deletions(-) diff --git a/src/Informedica.GenOrder.Lib/OrderVariable.fs b/src/Informedica.GenOrder.Lib/OrderVariable.fs index dc99323..49b1b85 100644 --- a/src/Informedica.GenOrder.Lib/OrderVariable.fs +++ b/src/Informedica.GenOrder.Lib/OrderVariable.fs @@ -129,7 +129,7 @@ module OrderVariable = else vr |> ValueRange.setOptVs ovar.Constraints.Values - |> Variable.setValueRange true ovar.Variable + |> Variable.setValueRange ovar.Variable } diff --git a/src/Informedica.GenSolver.Lib/Api.fs b/src/Informedica.GenSolver.Lib/Api.fs index acb88b2..4f2ff93 100644 --- a/src/Informedica.GenSolver.Lib/Api.fs +++ b/src/Informedica.GenSolver.Lib/Api.fs @@ -16,7 +16,11 @@ module Api = module Name = Variable.Name - /// Initialize the solver returning a set of equations + /// + /// Create a list of Equations from a list of strings + /// + /// List of strings + /// List of Equations let init eqs = let notEmpty = String.IsNullOrWhiteSpace >> not let prodEqs, sumEqs = eqs |> List.partition (String.contains "*") @@ -34,7 +38,17 @@ module Api = (parse prodEqs '*' |> createProdEqs) @ (parse sumEqs '+' |> createSumEqs) - let setVariableValues onlyMinIncrMax n p eqs = + /// + /// Apply a variable property to a list of equations + /// + /// Name of the variable + /// Property of the variable + /// List of equations + /// The variable option where the property is applied to + /// + /// The combination of n and p is equal to a constraint + /// + let setVariableValues n p eqs = eqs |> List.collect (Equation.findName n) |> function @@ -42,23 +56,27 @@ module Api = | var::_ -> p |> Property.toValueRange - |> Variable.setValueRange onlyMinIncrMax var + |> Variable.setValueRange var |> Some + /// Solve an `Equations` list let solveAll = Solver.solveAll + /// /// Solve an `Equations` list with - /// - /// * f: function used to process string message - /// * n: the name of the variable to be updated - /// * p: the property of the variable to be updated - /// * vs: the values to update the property of the variable - /// * eqs: the list of equations to solve + /// + /// True if only min, incr and max values are to be used + /// The algorithm to sort the equations + /// The logger to log operations + /// Name of the variable to be updated + /// Property of the variable to be updated + /// List of equations to solve + /// A result type of the solved equations let solve onlyMinIncrMax sortQue log n p eqs = eqs - |> setVariableValues onlyMinIncrMax n p + |> setVariableValues n p |> function | None -> eqs |> Ok | Some var -> @@ -66,16 +84,24 @@ module Api = |> Solver.solveVariable onlyMinIncrMax log sortQue var - /// Make a list of `EQD` - /// to contain only positive - /// values as solutions + /// + /// Set all variables in a list of equations to a non zero or negative value + /// + /// The list of Equations let nonZeroNegative eqs = eqs |> List.map Equation.nonZeroOrNegative - let applyConstraints onlyMinIncrMax log eqs cs = - let apply = Constraint.apply onlyMinIncrMax log + /// + /// Apply a list of constraints to a list of equations + /// + /// The logger to log operations + /// A list of Equations + /// A list of constraints + /// A list of Equations + let applyConstraints log eqs cs = + let apply = Constraint.apply log cs |> List.fold (fun acc c -> @@ -87,8 +113,16 @@ module Api = ) eqs + /// + /// Solve a list of equations using a list of constraints + /// + /// True if only min, incr and max values are to be used + /// The logger to log operations + /// A list of Equations + /// A list of constraints + /// A list of Equations let solveConstraints onlyMinIncrMax log cs eqs = cs |> Constraint.orderConstraints log - |> applyConstraints false log eqs + |> applyConstraints log eqs |> Solver.solveAll onlyMinIncrMax log diff --git a/src/Informedica.GenSolver.Lib/Constraint.fs b/src/Informedica.GenSolver.Lib/Constraint.fs index 06f64eb..9fcc1f9 100644 --- a/src/Informedica.GenSolver.Lib/Constraint.fs +++ b/src/Informedica.GenSolver.Lib/Constraint.fs @@ -12,12 +12,23 @@ module Constraint = module Name = Variable.Name + /// + /// Check whether constraint c1 has the same name as constraint c2. + /// let eqsName (c1 : Constraint) (c2 : Constraint) = c1.Name = c2.Name + /// + /// Print the constraint as a string + /// let toString { Name = n; Property = p } = $"{n |> Name.toString}: {p}" + /// + /// Give a constraint a score based on the property. + /// Low scores should be solved first. + /// + /// The constraint let scoreConstraint c = match c.Property with | ValsProp vs -> @@ -29,6 +40,11 @@ module Constraint = | _ -> -2, c + /// + /// Order constraints based on their score. + /// + /// The logger to log operations + /// The list of constraints let orderConstraints log cs = cs // calc min and max from valsprop constraints @@ -63,7 +79,15 @@ module Constraint = |> List.map snd - let apply onlyMinIncrMax log (c : Constraint) eqs = + /// + /// Apply a constraint to the matching variables + /// in the list of equations. + /// + /// The logger + /// The constraint + /// The list of Equations + /// The variable the constraint is applied to + let apply log (c : Constraint) eqs = eqs |> List.collect (Equation.findName c.Name) @@ -76,7 +100,7 @@ module Constraint = | vr::_ -> c.Property |> Property.toValueRange - |> Variable.setValueRange onlyMinIncrMax vr + |> Variable.setValueRange vr |> fun var -> c |> Events.ConstraintApplied @@ -85,8 +109,17 @@ module Constraint = var + /// + /// Apply a constraint to the matching variables and solve the + /// list of equations. + /// + /// Whether only min incr max should be calculated + /// The logger + /// The algorithm to sort the equations + /// The constraint + /// The list of Equations let solve onlyMinIncrMax log sortQue (c : Constraint) eqs = - let var = apply onlyMinIncrMax log c eqs + let var = apply log c eqs eqs |> Solver.solveVariable onlyMinIncrMax log sortQue var diff --git a/src/Informedica.GenSolver.Lib/Equation.fs b/src/Informedica.GenSolver.Lib/Equation.fs index b7a3e22..a156b49 100644 --- a/src/Informedica.GenSolver.Lib/Equation.fs +++ b/src/Informedica.GenSolver.Lib/Equation.fs @@ -336,8 +336,6 @@ 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 @@ -365,7 +363,7 @@ module Equation = None | Some var -> - let yNew = y <== var + let yNew = y @<- var if yNew <> y then // log finishing the calculation diff --git a/src/Informedica.GenSolver.Lib/Solver.fs b/src/Informedica.GenSolver.Lib/Solver.fs index 81b37d6..fb17f3c 100644 --- a/src/Informedica.GenSolver.Lib/Solver.fs +++ b/src/Informedica.GenSolver.Lib/Solver.fs @@ -78,17 +78,27 @@ module Solver = , rst - let sortQue onlyMinMax que = - if que |> List.length = 0 then que + /// + /// Sort a list of equations by the number of + /// total values in the equation. + /// + /// Whether only min incr max is calculated + /// The list of Equations + let sortQue onlyMinMax eqs = + if eqs |> List.length = 0 then eqs else - que + eqs |> List.sortBy (Equation.count onlyMinMax) //Equation.countProduct - /// Create the equation solver using a - /// product equation and a sum equation solver - /// and function to determine whether an - /// equation is solved + /// + /// Solve a set of equations. + /// + /// Whether to only use min, incr and max + /// The log function + /// The sort function for the que + /// An option variable to solve for + /// The equations to solve let solve onlyMinIncrMax log sortQue var eqs = let solveE n eqs eq = @@ -222,7 +232,14 @@ module Solver = Error (eqs, m) - //TODO: need to clean up the number check + /// + /// Solve a set of equations for a specific variable. + /// + /// Whether to only use min, incr and max + /// The log function + /// The sort function for the que + /// The variable to solve for + /// The equations to solve let solveVariable onlyMinIncrMax log sortQue vr eqs = let n1 = eqs |> List.length let solve = @@ -231,12 +248,18 @@ module Solver = match solve eqs with | Error (eqs, errs) -> Error (eqs, errs) | Ok eqs -> + //TODO: need to clean up the number check let n2 = eqs |> List.length if n2 <> n1 then failwith $"not the same number of eqs, was: {n1}, now {n2}" else Ok eqs - //TODO: need to clean up the number check + /// + /// Solve a set of equations for all variables. + /// + /// Whether to only use min, incr and max + /// The log function + /// The equations to solve let solveAll onlyMinIncrMax log eqs = let n1 = eqs |> List.length let solve = @@ -245,6 +268,7 @@ module Solver = match solve eqs with | Error (eqs, errs) -> Error (eqs, errs) | Ok eqs -> + //TODO: need to clean up the number check let n2 = eqs |> List.length if n2 <> n1 then failwith $"not the same number of eqs, was: {n1}, now {n2}" else Ok eqs diff --git a/src/Informedica.GenSolver.Lib/Types.fs b/src/Informedica.GenSolver.Lib/Types.fs index 36aa15a..d617829 100644 --- a/src/Informedica.GenSolver.Lib/Types.fs +++ b/src/Informedica.GenSolver.Lib/Types.fs @@ -124,8 +124,8 @@ module rec Types = /// /// Represents a constraint on a `Variable`. - /// I.e. either a set of values, or an increment - /// or a minimum of maximum. + /// I.e. either a set of values, or an increment, + /// minimum or maximum. /// type Constraint = { diff --git a/src/Informedica.GenSolver.Lib/Variable.fs b/src/Informedica.GenSolver.Lib/Variable.fs index 3203524..fd74810 100644 --- a/src/Informedica.GenSolver.Lib/Variable.fs +++ b/src/Informedica.GenSolver.Lib/Variable.fs @@ -2989,8 +2989,7 @@ module Variable = /// /// let applyExpr y expr = - let appl _ get set vr = - //printfn $"{s}" + let appl get set vr = match expr |> get with | Some m -> vr |> set m | None -> vr @@ -3000,9 +2999,9 @@ module Variable = | ValSet vs -> y |> setValueSet vs | _ -> y - |> appl "incr" getIncr setIncr - |> appl "min" getMin setMin - |> appl "max" getMax setMax + |> appl getIncr setIncr + |> appl getMin setMin + |> appl getMax setMax module Operators = @@ -3015,8 +3014,6 @@ module Variable = let inline (^-) vr1 vr2 = calc false (-) (vr1, vr2) - let inline (^<-) vr1 vr2 = applyExpr vr1 vr2 - let inline (@*) vr1 vr2 = calc true (*) (vr1, vr2) @@ -3093,14 +3090,11 @@ module Variable = /// Apply a `ValueRange` **vr** to /// `Variable` **v**. - let setValueRange onlyMinIncrMax var vr = - let op = - if onlyMinIncrMax then (@<-) else (^<-) - + let setValueRange var vr = try { var with Values = - (var |> get).Values |> op <| vr + (var |> get).Values @<- vr } with @@ -3239,8 +3233,6 @@ module Variable = let inline (^-) vr1 vr2 = calc (^-) (vr1, vr2) - let inline (^<-) vr1 vr2 = vr2 |> getValueRange |> setValueRange false vr1 - let inline (@*) vr1 vr2 = calc (@*) (vr1, vr2) @@ -3250,7 +3242,7 @@ module Variable = let inline (@-) vr1 vr2 = calc (@-) (vr1, vr2) - let inline (@<-) vr1 vr2 = vr2 |> getValueRange |> setValueRange true vr1 + let inline (@<-) vr1 vr2 = vr2 |> getValueRange |> setValueRange vr1 /// Constant 1 diff --git a/tests/Informedica.GenSolver.Tests/Tests.fs b/tests/Informedica.GenSolver.Tests/Tests.fs index 1ebc250..3a37cf9 100644 --- a/tests/Informedica.GenSolver.Tests/Tests.fs +++ b/tests/Informedica.GenSolver.Tests/Tests.fs @@ -128,7 +128,7 @@ module TestSolver = let setProp n p eqs = let n = n |> Name.createExc - match eqs |> Api.setVariableValues true n p with + match eqs |> Api.setVariableValues n p with | Some var -> eqs |> List.map (fun e ->