From ce32d45ecb7c701b5545fe33f6a800aea5767b0b Mon Sep 17 00:00:00 2001 From: Fredrik Bagge Carlson Date: Fri, 8 May 2026 06:56:21 +0200 Subject: [PATCH 1/2] validate `external_inputs`/`external_outputs` names in `connect` (#114) Names not present in the assembled system used to silently prefix-match internal ports, producing the wrong external inputs. Error instead. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/named_systems2.jl | 9 +++++++++ test/test_named_systems2.jl | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/named_systems2.jl b/src/named_systems2.jl index ebe0e210..87229aa4 100644 --- a/src/named_systems2.jl +++ b/src/named_systems2.jl @@ -675,6 +675,15 @@ function connect(systems; u1::Vector{Symbol}, y1::Vector{Symbol}, external_input check_unique(full.y, "system outputs") + if w1 isa AbstractVector{Symbol} + unknown_w1 = setdiff(w1, full.u) + isempty(unknown_w1) || error("The following names in `external_inputs` were not found among the system inputs: $unknown_w1. Available inputs are $(full.u). To expose a fresh external signal, either rename an existing input port to that name, or insert a `splitter(name, n)` and connect its outputs.") + end + if z1 isa AbstractVector{Symbol} + unknown_z1 = setdiff(z1, full.y) + isempty(unknown_z1) || error("The following names in `external_outputs` were not found among the system outputs: $unknown_z1. Available outputs are $(full.y).") + end + if verbose leftover_inputs = setdiff(full.u, [u1; w1]) isempty(leftover_inputs) || @warn("The following inputs were unconnected $leftover_inputs, ignore this warning if you rely on prefix matching. Turn off this warning by passing `verbose = false`.") diff --git a/test/test_named_systems2.jl b/test/test_named_systems2.jl index b3e1cdb5..98555277 100644 --- a/test/test_named_systems2.jl +++ b/test/test_named_systems2.jl @@ -398,6 +398,28 @@ nss = named_ss(tf(1, [1,1])) @test nss isa NamedStateSpace +## Issue #114 — `connect` must reject `external_inputs`/`external_outputs` names +## that don't exist among the assembled inputs/outputs. Previously the names +## silently prefix-matched against internal ports and produced wrong externals. +sys_y = named_ss(ss(-1.0), u=:yinu, y=:yiny) +sys_in = named_ss(ss(1.0), u=:in, y=:out) + +# :y is supposed to be external but isn't in full.u; would have prefix-matched [:yinu, :yiny] +@test_throws "external_inputs" connect( + [sys_y, sys_in], + [:out => :in]; + external_inputs = [:y], + external_outputs = [:yiny], +) + +# Symmetric: external_outputs name not in full.y +@test_throws "external_outputs" connect( + [sys_y, sys_in], + [:out => :in]; + external_inputs = [:yinu], + external_outputs = [:bogus], +) + ## Test where one external input goes to several inputs with the same name, with and without using a splitter s1 = ssrand(1,2,2) From b1c088dda9c6cbbc40e5919f3d3f6817c6060495 Mon Sep 17 00:00:00 2001 From: Fredrik Bagge Carlson Date: Fri, 8 May 2026 08:43:26 +0200 Subject: [PATCH 2/2] narrow `connect` validation to ambiguous prefix matches Single-prefix matches like `:x` -> `Symbol("x(t)")` are still accepted (LQG tests rely on this). Only multi-result prefix expansion or no-match at all errors now. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/named_systems2.jl | 25 +++++++++++++++------- test/test_named_systems2.jl | 42 ++++++++++++++++++++++++------------- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/src/named_systems2.jl b/src/named_systems2.jl index 87229aa4..2e71041e 100644 --- a/src/named_systems2.jl +++ b/src/named_systems2.jl @@ -17,6 +17,21 @@ function check_unique(vals, s, msg=""; throw = true) return true end +_check_external_names(::Any, _allnames, _argname, _label, _hint) = nothing +function _check_external_names(names::AbstractVector{Symbol}, allnames, argname, label, hint) + snames = string.(allnames) + for name in names + name in allnames && continue + matches = allnames[findall(startswith(string(name)), snames)] + if isempty(matches) + error("The name $name in `$argname` was not found among the $label. Available are $(allnames)$hint.") + elseif length(matches) > 1 + error("The name $name in `$argname` is ambiguous: it prefix-matches multiple $label $matches. Use the exact name to disambiguate$hint.") + end + end + return nothing +end + function check_all_unique(s1, s2; throw=true) valx = check_unique([getproperty(s1, :x); getproperty(s2, :x)], "x"; throw) check_unique([getproperty(s1, :u); getproperty(s2, :u)], "u"; throw=true) @@ -675,14 +690,8 @@ function connect(systems; u1::Vector{Symbol}, y1::Vector{Symbol}, external_input check_unique(full.y, "system outputs") - if w1 isa AbstractVector{Symbol} - unknown_w1 = setdiff(w1, full.u) - isempty(unknown_w1) || error("The following names in `external_inputs` were not found among the system inputs: $unknown_w1. Available inputs are $(full.u). To expose a fresh external signal, either rename an existing input port to that name, or insert a `splitter(name, n)` and connect its outputs.") - end - if z1 isa AbstractVector{Symbol} - unknown_z1 = setdiff(z1, full.y) - isempty(unknown_z1) || error("The following names in `external_outputs` were not found among the system outputs: $unknown_z1. Available outputs are $(full.y).") - end + _check_external_names(w1, full.u, "external_inputs", "system inputs", "; insert a `splitter(name, n)` to expose a fresh external signal connected to multiple ports") + _check_external_names(z1, full.y, "external_outputs", "system outputs", "") if verbose leftover_inputs = setdiff(full.u, [u1; w1]) diff --git a/test/test_named_systems2.jl b/test/test_named_systems2.jl index 98555277..fc55b537 100644 --- a/test/test_named_systems2.jl +++ b/test/test_named_systems2.jl @@ -398,28 +398,42 @@ nss = named_ss(tf(1, [1,1])) @test nss isa NamedStateSpace -## Issue #114 — `connect` must reject `external_inputs`/`external_outputs` names -## that don't exist among the assembled inputs/outputs. Previously the names -## silently prefix-matched against internal ports and produced wrong externals. -sys_y = named_ss(ss(-1.0), u=:yinu, y=:yiny) -sys_in = named_ss(ss(1.0), u=:in, y=:out) +## Issue #114 — `connect` must reject ambiguous or unknown +## `external_inputs`/`external_outputs` names. Previously such names silently +## prefix-matched multiple internal ports and produced wrong externals. +sys_y1 = named_ss(ss(-1.0), u=:yinu, y=:youta) +sys_y2 = named_ss(ss( 1.0), u=:yiny, y=:youtb) -# :y is supposed to be external but isn't in full.u; would have prefix-matched [:yinu, :yiny] +# :y is supposed to be external but prefix-matches BOTH :yinu and :yiny @test_throws "external_inputs" connect( - [sys_y, sys_in], - [:out => :in]; + [sys_y1, sys_y2], + Pair{Symbol,Symbol}[]; external_inputs = [:y], - external_outputs = [:yiny], + external_outputs = [:youta, :youtb], ) -# Symmetric: external_outputs name not in full.y +# Symmetric: :yout prefix-matches both :youta and :youtb @test_throws "external_outputs" connect( - [sys_y, sys_in], - [:out => :in]; - external_inputs = [:yinu], - external_outputs = [:bogus], + [sys_y1, sys_y2], + Pair{Symbol,Symbol}[]; + external_inputs = [:yinu, :yiny], + external_outputs = [:yout], ) +# Unknown name (no exact, no prefix) errors too +@test_throws "external_inputs" connect( + [sys_y1, sys_y2], + Pair{Symbol,Symbol}[]; + external_inputs = [:totally_bogus], + external_outputs = [:youta, :youtb], +) + +# Single-prefix match (e.g. `:x` -> `Symbol("x(t)")`) must still work — used by LQG tests +sys_a = named_ss(ssrand(1, 1, 1), u=Symbol("x(t)"), y=Symbol("y(t)"), x=:xa) +sys_b = named_ss(ssrand(1, 1, 1), u=:u_b, y=:y_b, x=:xb) +@test connect([sys_a, sys_b], [Symbol("y(t)") => :u_b]; + external_inputs = [:x], external_outputs = [:y_b]) isa NamedStateSpace + ## Test where one external input goes to several inputs with the same name, with and without using a splitter s1 = ssrand(1,2,2)