Merge pull request #577 from LH-and-FPGA/Debug2-PS

Debug2 ps

Author: LH-and-FPGA

docs/index.md

@@ -11,6 +11,8 @@ See [Getting Started](gettingStarted.html) for how to put Issie on your laptop o
 
 See [User Tutorial](userGuide.html) for a useful introduction to Issie on one page which you can follow or read.
 
+See [Parameter System](parameterSystem.html) for a detailed explanation of symbolic parameters, expressions, constraints, and simulation integration.
+
 <br>
 
 ## What is ISSIE?

docs/parameterSystem.md

@@ -206,6 +206,12 @@ The parser uses recursive descent with separate functions for each precedence le
 - `parseFactors`: Processes multiplication, division, modulo
 - `parseExpressionTokens`: Handles addition and subtraction
 
+Notes and caveats:
+- Tokenizer restricts inputs to digits/letters/operators/whitespace; unsupported characters are reported precisely.
+- Division and modulo are evaluated during constant-folding; add a MinVal constraint to prevent zero divisors where needed.
+  
+Code: `src/Renderer/Common/ParameterTypes.fs` (`parseExpression`, tokenizer regex, and helpers)
+
 ## Parameter Scoping & Precedence
 
 ### Scope Levels
@@ -474,6 +480,41 @@ resolveParametersForComponent: ParamBindings -> Map<ParamSlot, ConstrainedExpr>
 evaluateConstraints: ParamBindings -> ConstrainedExpr list -> (Msg -> unit) -> Result<Unit, ParamConstraint list>
 ```
 
+## Resolution Mechanics Deep-Dive
+
+- UI evaluation: `ParameterTypes.evaluateParamExpression` performs recursive substitution and constant-folding with detailed errors. Used by `ParameterView` for validation and preview.
+- Graph evaluation: `GraphMerger.resolveParametersInSimulationGraph` uses internal `evalExpr` (returns `Option<int>`) and `applySlotValue` to write concrete values into `SimulationGraph` component types after merge.
+- Validation evaluation: `CanvasStateAnalyser.checkCustomComponentForOkIOs` embeds a minimal evaluator supporting only `PInt` and `PParameter` to resolve port widths quickly for label checking.
+- Slot access: Lenses `ParameterView.compSlot_` and `ParameterView.modelToSlot_` provide strongly typed access into `Component.Type` for `Buswidth`, `NGateInputs`, and `IO label`.
+
+## Developer Notes (Files & Responsibilities)
+
+- `src/Renderer/Common/ParameterTypes.fs`: Types (`ParamExpression`, `ParamConstraint`, `ParamSlot`, `ParameterDefs`), parser (`parseExpression`), evaluator (`evaluateParamExpression`), renderer (`renderParamExpression`).
+- `src/Renderer/UI/ParameterView.fs`: Sheet defaults and slot bindings CRUD, constraint checking, component updates, and parameter UI fields/popups.
+- `src/Renderer/UI/CatalogueView.fs`: Merges parent sheet defaults with sub-sheet defaults, resolves canvas before extracting `InputLabels`/`OutputLabels`, sets `ParameterBindings` on instances.
+- `src/Renderer/Simulator/GraphMerger.fs`: Two-stage resolution during merge; instance bindings first, then sheet defaults; recursion into nested custom components.
+- `src/Renderer/Simulator/CanvasStateAnalyser.fs`: Lightweight parameter resolution for port label validation.
+
+## Development History
+
+Key commits that shaped the current system (from `git log`):
+- a67fa72f Fix parameter resolution in simulation graph creation
+  - Passes `loadedDependencies` into merger; applies instance-specific `ParameterBindings` for custom components.
+- 83bb0b0b Fix forward reference issue in parameter resolution
+  - Introduces two-stage resolution: resolve custom component instance bindings first, then sheet-level defaults.
+- b510fe4b Parameter System Redo
+  - Reworks UI binding flow and simulation integration; clearer separation of concerns.
+- edf61e87 Parameter System Support
+  - Integrates `ParameterTypes.fs`, updates merger/validation, and adds comprehensive documentation.
+
+For a side-by-side comparison with an earlier streamlined approach, see `PARAMETER_SYSTEM_COMPARISON.md`.
+
+## Known Limitations
+
+- Integer-only parameters today (`ParamInt = int`); very large constants may require future `bigint`.
+- No explicit guard on divide/modulo by zero during constant-folding; enforce with constraints.
+- Parameter names are unqualified; deeper inheritance across sheet hierarchies may require qualification if extended.
+
 ## Best Practices
 
 1. **Use descriptive parameter names**: `DATA_WIDTH` instead of `W`
@@ -504,4 +545,4 @@ evaluateConstraints: ParamBindings -> ConstrainedExpr list -> (Msg -> unit) -> R
 ### Simulation Failure
 - Verify all parameters resolve to valid integers
 - Check for circular parameter dependencies
-- Ensure component types match parameter slots
+- Ensure component types match parameter slots

src/Renderer/Common/ParameterTypes.fs

@@ -49,6 +49,9 @@ type CompSlotName =
     | NGateInputs
     | IO of Label: string
     | CustomCompParam of ParamName: string // TODO: implement this case
+    // SplitN-specific parameterised slots
+    | SplitNWidth of Index: int
+    | SplitNLSB of Index: int
 
 /// A slot in a component instance that can be bound to a parameter expression
 /// CompId should be a ComponentId but then we would need these types to be defined after CommonTypes.

src/Renderer/Simulator/GraphMerger.fs

@@ -428,6 +428,13 @@ let rec resolveParametersInSimulationGraph
         // IO ports
         | IO _, Input1 (_, dv) -> Input1 (value, dv)
         | IO _, Output _ -> Output value
+        // SplitN output slots
+        | SplitNWidth idx, SplitN (n, widths, lsbs) when idx >= 0 && idx < List.length widths ->
+            let newWidths = widths |> List.mapi (fun i w -> if i = idx then value else w)
+            SplitN (n, newWidths, lsbs)
+        | SplitNLSB idx, SplitN (n, widths, lsbs) when idx >= 0 && idx < List.length lsbs ->
+            let newLsbs = lsbs |> List.mapi (fun i l -> if i = idx then value else l)
+            SplitN (n, widths, newLsbs)
         | _ -> compType
 
     // Process a single component
@@ -442,12 +449,13 @@ let rec resolveParametersInSimulationGraph
                 | Some value -> 
                     Ok { c with Type = applySlotValue c.Type slot.CompSlot value }
                 | None ->
-                    Error ({ 
+                    let err: SimGraphTypes.SimulationError = {
                         ErrType = GenericSimError "Parameter expression could not be fully evaluated"
                         InDependency = Some currDiagramName
                         ComponentsAffected = [compId]
-                        ConnectionsAffected = [] 
-                    }: SimulationError)
+                        ConnectionsAffected = []
+                    }
+                    Error err
             )
         ) (Ok comp)
 

src/Renderer/UI/ParameterView.fs

@@ -101,6 +101,20 @@ let compSlot_ (compSlotName:CompSlotName) : Optics.Lens<Component, int> =
                 | Input1 (busWidth, _) -> busWidth
                 | Output busWidth -> busWidth
                 | _ -> failwithf $"Invalid component {comp.Type} for IO"
+            | SplitNWidth idx ->
+                match comp.Type with
+                | SplitN (_, widths, _) ->
+                    if idx >= 0 && idx < List.length widths then
+                        widths[idx]
+                    else failwithf $"SplitNWidth index %d{idx} out of range"
+                | _ -> failwithf $"Invalid component {comp.Type} for SplitNWidth"
+            | SplitNLSB idx ->
+                match comp.Type with
+                | SplitN (_, _, lsbs) ->
+                    if idx >= 0 && idx < List.length lsbs then
+                        lsbs[idx]
+                    else failwithf $"SplitNLSB index %d{idx} out of range"
+                | _ -> failwithf $"Invalid component {comp.Type} for SplitNLSB"
             | CustomCompParam paramName ->
                 match comp.Type with
                 | Custom customComp ->
@@ -152,6 +166,20 @@ let compSlot_ (compSlotName:CompSlotName) : Optics.Lens<Component, int> =
                         | Input1 (_, defaultValue) -> Input1 (value, defaultValue)
                         | Output _ -> Output value
                         | _ -> failwithf $"Invalid component {comp.Type} for IO"
+                    | SplitNWidth idx ->
+                        match comp.Type with
+                        | SplitN (n, widths, lsbs) ->
+                            if idx < 0 || idx >= List.length widths then failwithf $"SplitNWidth index %d{idx} out of range"
+                            let newWidths = widths |> List.mapi (fun i w -> if i = idx then value else w)
+                            SplitN (n, newWidths, lsbs)
+                        | _ -> failwithf $"Invalid component {comp.Type} for SplitNWidth"
+                    | SplitNLSB idx ->
+                        match comp.Type with
+                        | SplitN (n, widths, lsbs) ->
+                            if idx < 0 || idx >= List.length lsbs then failwithf $"SplitNLSB index %d{idx} out of range"
+                            let newLsbs = lsbs |> List.mapi (fun i l -> if i = idx then value else l)
+                            SplitN (n, widths, newLsbs)
+                        | _ -> failwithf $"Invalid component {comp.Type} for SplitNLSB"
                     | CustomCompParam paramName ->
                         match comp.Type with
                         | Custom customComp ->
@@ -232,8 +260,8 @@ let evaluateConstraints
     else Error result
 
 
-/// Generates a ParameterExpression from input text
-/// Operators are left-associative
+// Generates a ParameterExpression from input text
+// Operators are left-associative
 // parseExpression has been moved to ParameterTypes module
 
 
@@ -306,6 +334,20 @@ let updateComponent dispatch model slot value =
         match comp.Type with
         | GateN (gateType, _) -> model.Sheet.ChangeGate sheetDispatch compId gateType value
         | _ -> failwithf $"Gate cannot have type {comp.Type}"
+    | SplitNWidth idx ->
+        match comp.Type with
+        | SplitN (n, widths, lsbs) ->
+            if idx < 0 || idx >= List.length widths then failwithf $"SplitNWidth index %d{idx} out of range"
+            let newWidths = widths |> List.mapi (fun i w -> if i = idx then value else w)
+            model.Sheet.ChangeSplitN sheetDispatch compId n newWidths lsbs
+        | _ -> failwithf $"SplitNWidth cannot be applied to {comp.Type}"
+    | SplitNLSB idx ->
+        match comp.Type with
+        | SplitN (n, widths, lsbs) ->
+            if idx < 0 || idx >= List.length lsbs then failwithf $"SplitNLSB index %d{idx} out of range"
+            let newLsbs = lsbs |> List.mapi (fun i l -> if i = idx then value else l)
+            model.Sheet.ChangeSplitN sheetDispatch compId n widths newLsbs
+        | _ -> failwithf $"SplitNLSB cannot be applied to {comp.Type}"
     | CustomCompParam paramName ->
         // For custom component parameters, we need to update the parameter bindings
         match comp.Type with
@@ -885,7 +927,19 @@ let resolveParametersForComponent
                             | Buswidth -> currentType |> (evaluatedValue ^= buswidthPrism)
                             | NGateInputs -> currentType |> (evaluatedValue ^= ngateInputsPrism)
                             | IO _ -> currentType |> (evaluatedValue ^= ioPortPrism)
-                            | _ -> currentType
+                            | SplitNWidth idx ->
+                                match currentType with
+                                | SplitN (n, widths, lsbs) when idx >= 0 && idx < List.length widths ->
+                                    let newWidths = widths |> List.mapi (fun i w -> if i = idx then evaluatedValue else w)
+                                    SplitN (n, newWidths, lsbs)
+                                | _ -> currentType
+                            | SplitNLSB idx ->
+                                match currentType with
+                                | SplitN (n, widths, lsbs) when idx >= 0 && idx < List.length lsbs ->
+                                    let newLsbs = lsbs |> List.mapi (fun i l -> if i = idx then evaluatedValue else l)
+                                    SplitN (n, widths, newLsbs)
+                                | _ -> currentType
+                            | CustomCompParam _ -> currentType
                         (newType, None)
                     | Error err -> (currentType, Some err)
             )
@@ -1127,6 +1181,8 @@ let private makeSlotsField (model: ModelType.Model) (comp:LoadedComponent) dispa
             | Buswidth -> "Buswidth"
             | NGateInputs -> "Num inputs"
             | IO label -> $"Input/output {label}"
+            | SplitNWidth idx -> $"SplitN output {idx} width"
+            | SplitNLSB idx -> $"SplitN output {idx} LSB"
             | CustomCompParam paramName -> $"Custom parameter {paramName}"
 
         let name = if Map.containsKey (ComponentId slot.CompId) model.Sheet.Wire.Symbol.Symbols then

src/Renderer/UI/SelectedComponentView.fs

@@ -574,41 +574,48 @@ let private changeSplitN model (comp:Component) dispatch =
             [Style [Color Red]]
             [str errText]
 
-        let constraints = [
-            MinVal (PInt 2, "Must have at least 2 outputs")
-            MaxVal (PInt Constants.maxSplitMergeBranches, $"Cannot have more than {Constants.maxSplitMergeBranches} outputs")
-        ]
-        
-        ParameterView.paramInputField model title 2 (Some nInp) constraints (Some comp) NGateInputs dispatch
-        div [Style [Display DisplayOptions.Flex; MarginLeft "180px"]] [
-        // Add headers for the "Width" and "LSB" columns
-            Label.label [Label.Props [Style [TextAlign TextAlignOptions.Center; MarginRight "20px"]]] [str "Width"]
-            Label.label [Label.Props [Style [TextAlign TextAlignOptions.Center; MarginLeft "20px"]]] [str "LSB"]
+        // Plain numeric input for number of outputs (no parameter box)
+        Field.div [] [
+            Label.label [] [ str title ]
+            Input.number [
+                Input.Props [ Style [ Width "80px" ]; Min 2; Max Constants.maxSplitMergeBranches ]
+                Input.DefaultValue (string nInp)
+                Input.OnChange (getIntEventValue >> fun newNum ->
+                    let newWidths = changeWidths widths newNum 1
+                    let newLsbs = changeLsbs lsbs newWidths newNum
+                    model.Sheet.ChangeSplitN sheetDispatch (ComponentId comp.Id) newNum newWidths newLsbs)
+            ]
         ]
-        List.mapi2 (fun index defaultWidth defaultLsb ->
-            let portTitle = 
-                match defaultWidth with
-                | n when n > 1 -> sprintf "Output Port %d" index
-                | _ -> sprintf "Output Port %d" index
-            let bits = 
-                match defaultWidth with
-                | n when n > 1 -> sprintf "(%d:%d)" (n+defaultLsb-1) defaultLsb
-                | _ -> sprintf "(%d)" defaultLsb
-            intFormField2 portTitle bits "60px" defaultWidth defaultLsb 1 0 
-                (fun newWidth -> 
-                    let neWidths = 
-                        widths
-                        |> List.mapi (fun i x -> if i = index then newWidth else x)
-                    model.Sheet.ChangeSplitN sheetDispatch (ComponentId comp.Id) nInp neWidths lsbs
-                    )
-                (fun lsb -> 
-                    let newLsbs = 
-                        lsbs
-                        |> List.mapi (fun i x -> if i = index then lsb else x)
-                    model.Sheet.ChangeSplitN sheetDispatch (ComponentId comp.Id) nInp widths newLsbs
-                    )
-        ) widths lsbs
-        |> div [Style [MarginBottom "20px"]] 
+
+        // Parameter boxes for each output's Width and LSB
+        widths
+        |> List.mapi (fun index defaultWidth ->
+            let defaultLsb = lsbs.[index]
+            div [ Style [ MarginLeft "10px"; MarginBottom "10px" ] ] [
+                Label.label [] [ str (sprintf "Output Port %d" index) ]
+                // Width parameter
+                ParameterView.paramInputField 
+                    model 
+                    "Width" 
+                    1 
+                    (Some defaultWidth) 
+                    [ MinVal (PInt 1, "Width must be at least 1") ]
+                    (Some comp) 
+                    (SplitNWidth index) 
+                    dispatch
+                // LSB parameter
+                ParameterView.paramInputField 
+                    model 
+                    "LSB" 
+                    0 
+                    (Some defaultLsb) 
+                    [ MinVal (PInt 0, "LSB must be non-negative") ]
+                    (Some comp) 
+                    (SplitNLSB index) 
+                    dispatch
+            ]
+        )
+        |> div [Style [MarginBottom "20px"]]
     ]
 
 

src/Renderer/UI/Style.fs

@@ -220,10 +220,17 @@ let ttGridHiddenColumnProps gridWidth= [
 ]
 
 let ttGridContainerStyle model = 
-    let widthRightSec = rightSectionWidth model
+    // Compute number of visible columns (total IOs minus hidden columns)
+    let totalCols = model.TTConfig.IOOrder.Length
+    let hiddenCols = model.TTConfig.HiddenColumns.Length
+    // Ensure at least 1 to avoid invalid CSS when table is empty
+    let visibleCols = max 1 (totalCols - hiddenCols)
     Style [
         Display DisplayOptions.Grid
         GridAutoFlow "column"
+        // Make grid span the container width and distribute columns evenly
+        Width "100%"
+        GridTemplateColumns (sprintf "repeat(%d, minmax(0, 1fr))" visibleCols)
     ]
 
 

src/Renderer/UI/TruthTable/TruthTableUpdate.fs

@@ -250,7 +250,12 @@ let truthTableUpdate (model: Model) (msg:TTMsg) : (Model * Cmd<Msg>)  =
         /// Recursive function to hide columns and adjust the positions of the remaining
         /// visible columns.
         let rec correctProps (index: int) (acc: list<CellIO*list<CSSProp>>) (lst: CellIO list):  list<CellIO*list<CSSProp>>=
-            let hiddenProps = ttGridHiddenColumnProps model.TTConfig.IOOrder.Length
+            // Number of visible columns is total minus hidden
+            let visibleCount =
+                (Array.toList model.TTConfig.IOOrder
+                 |> List.except model.TTConfig.HiddenColumns
+                 |> List.length)
+            let hiddenProps = ttGridHiddenColumnProps visibleCount
             match lst with
             | [] -> acc
             | io::tl ->