Skip to content

Use oneunit in remaining tf/ss/zpk type-promotion sites#1055

Merged
baggepinnen merged 1 commit into
JuliaControl:masterfrom
baggepinnen:fix-measurement-conversion-oneunit
May 14, 2026
Merged

Use oneunit in remaining tf/ss/zpk type-promotion sites#1055
baggepinnen merged 1 commit into
JuliaControl:masterfrom
baggepinnen:fix-measurement-conversion-oneunit

Conversation

@baggepinnen
Copy link
Copy Markdown
Member

Summary

  • Completes the fix started in b394b96. Measurements.jl deliberately defines Base.one(::Type{Measurement{T}}) = one(T), so one(Measurement{Float64}) === 1.0::Float64. Five conversion sites still inferred the result element type via typeof(one(T0)/one(T0)) / typeof(zero(T0)/one(T0)), which collapsed Measurement{Float64} to Float64 and triggered the MethodError: no method matching Float64(::Measurement{Float64}) reported in MatrixPencils - a new feature #1027 along the tf → zpk, ss → tf, ss → zpk, minreal, and charpoly paths.
  • Switched the remaining sites to typeof(oneunit(T)/oneunit(T)), matching the existing fix in convert(::Type{StateSpace}, G::TransferFunction). Behavior for Int/Float64/BigFloat/etc. is unchanged (oneunit/oneunit reduces to one/one when one(T) isa T).
  • Sites touched: types/conversion.jl (×3: ss→TF/SisoRational, ss→TF/SisoZpk, charpoly), types/zpk.jl (tf→zpk), types/SisoTfTypes/SisoRational.jl (minreal), matrix_comps.jl (_infnorm_two_steps_dt, for consistency).

Closes #1027 to the extent it's a ControlSystems issue. (zpk and tf(::ss{Measurement}) may still fail downstream inside Polynomials.roots / LinearAlgebra.hessenberg, which need BLAS-able elements — separate upstream limitations.)

Test plan

  • lib/ControlSystemsBase test suite: 21146 pass, 6 broken (unchanged from master).
  • RobustAndOptimalControl.jl test suite (extra promotion coverage): 1357 pass, 6 broken (unchanged).
  • Issue MatrixPencils - a new feature #1027 reproducer: ss(P) now returns StateSpace{Continuous, Measurement{Float64}} with elements preserved.

🤖 Generated with Claude Code

Completes b394b96: five more `typeof(one(T)/one(T))` sites still
collapsed `Measurement{Float64}` to `Float64` (since Measurements.jl
defines `one(Measurement{T}) = one(T)`), breaking the tf->zpk, ss->tf,
ss->zpk, minreal, and charpoly conversion paths reported in JuliaControl#1027.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.61%. Comparing base (75801d6) to head (b0b103b).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1055      +/-   ##
==========================================
+ Coverage   91.57%   91.61%   +0.04%     
==========================================
  Files          42       42              
  Lines        5623     5653      +30     
==========================================
+ Hits         5149     5179      +30     
  Misses        474      474              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@baggepinnen baggepinnen merged commit 0aa6e4c into JuliaControl:master May 14, 2026
6 checks passed
@baggepinnen baggepinnen deleted the fix-measurement-conversion-oneunit branch May 14, 2026 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MatrixPencils - a new feature

1 participant