Skip to content

Commit 80019f3

Browse files
baggepinnenbaggepinnen
committed
fixes
1 parent 01ab50e commit 80019f3

File tree

3 files changed

+29
-67
lines changed

3 files changed

+29
-67
lines changed

‎lib/ControlSystemsBase/ext/ControlSystemsBaseMakieExt.jl‎

Lines changed: 26 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -14,36 +14,12 @@ using ControlSystemsBase: downsample, _processfreqplot, _default_freq_vector,
1414
sisomargin, relative_gain_array, rlocus,
1515
input_names, output_names, state_names, system_name,
1616
iscontinuous, isdiscrete, issiso, isrational,
17-
integrator_excess, balance_statespace, LTISystem
17+
integrator_excess, balance_statespace, LTISystem,
18+
_PlotScale, _PlotScaleFunc, _PlotScaleStr, _span # Use existing plot scale settings
1819

19-
# Global plot scale settings (matching Plots.jl version)
20-
const _PlotScale = Ref("log10")
21-
const _PlotScaleFunc = Ref(:log10)
22-
const _PlotScaleStr = Ref("")
23-
24-
"""
25-
CSMakie.setPlotScale(str)
26-
27-
Set the default scale of magnitude in `bodeplot` and `sigmaplot` for Makie plots.
28-
`str` should be either `"dB"` or `"log10"`. The default scale if none is chosen is `"log10"`.
29-
"""
30-
function CSMakie.setPlotScale(str::AbstractString)
31-
if str == "dB"
32-
_PlotScale[] = str
33-
_PlotScaleFunc[] = :identity
34-
_PlotScaleStr[] = "(dB)"
35-
elseif str == "log10"
36-
_PlotScale[] = str
37-
_PlotScaleFunc[] = :log10
38-
_PlotScaleStr[] = ""
39-
else
40-
error("Scale must be set to either \"dB\" or \"log10\"")
41-
end
42-
end
43-
44-
# Helper function to get y-scale transform
20+
# Helper function to get y-scale transform for Makie
4521
function get_yscale_transform()
46-
_PlotScaleFunc[] == :log10 ? Makie.log10 : identity
22+
ControlSystemsBase._PlotScaleFunc == :log10 ? Makie.log10 : identity
4723
end
4824

4925
# ====== PZMap (Pole-Zero Map) ======
@@ -163,7 +139,7 @@ function CSMakie.bodeplot(systems::Union{LTISystem, AbstractVector{<:LTISystem}}
163139
xscale = log10,
164140
yscale = get_yscale_transform(),
165141
xlabel = plotphase ? "" : (hz ? "Frequency [Hz]" : "Frequency [rad/s]"),
166-
ylabel = i == 1 ? "Magnitude $_PlotScaleStr[]" : "",
142+
ylabel = i == 1 ? "Magnitude $(ControlSystemsBase._PlotScaleStr)" : "",
167143
title = j == 1 && i == 1 ? "Bode Plot" : "")
168144
axes_mag[i, j] = ax_mag
169145

@@ -190,7 +166,7 @@ function CSMakie.bodeplot(systems::Union{LTISystem, AbstractVector{<:LTISystem}}
190166

191167
mag, phase = bode(sbal, w; unwrap=false)
192168

193-
if _PlotScale[] == "dB"
169+
if ControlSystemsBase._PlotScale == "dB"
194170
mag = 20*log10.(mag)
195171
elseif 0 ∈ mag
196172
replace!(mag, 0 => NaN)
@@ -208,8 +184,7 @@ function CSMakie.bodeplot(systems::Union{LTISystem, AbstractVector{<:LTISystem}}
208184
lab = _get_plotlabel(s, i, j)
209185

210186
if adaptive && eltype(magdata) <: AbstractFloat
211-
lmag = _PlotScale[] == "dB" ? magdata : log.(magdata)
212-
wsi, magi, _ = downsample(ws, lmag, maximum(lmag) - minimum(lmag)/500)
187+
wsi, magi, _ = downsample(ws, magdata, _span(magdata)/500)
213188
lines!(ax_mag, wsi, magi, label=lab)
214189
else
215190
lines!(ax_mag, ws, magdata, label=lab)
@@ -231,7 +206,7 @@ function CSMakie.bodeplot(systems::Union{LTISystem, AbstractVector{<:LTISystem}}
231206
ax_phase = axes_phase[i, j]
232207
if adaptive && eltype(phasedata) <: AbstractFloat
233208
wsp, phasep, _ = downsample(ws, phasedata,
234-
maximum(phasedata) - minimum(phasedata)/500)
209+
_span(phasedata)/500)
235210
lines!(ax_phase, wsp, phasep)
236211
else
237212
lines!(ax_phase, ws, phasedata)
@@ -291,8 +266,8 @@ function CSMakie.nyquistplot(systems::Union{LTISystem, AbstractVector{<:LTISyste
291266
lab = _get_plotlabel(s, i, j)
292267

293268
if adaptive
294-
indsre, _, _ = downsample(w, redata, 1/500)
295-
indsim, _, _ = downsample(w, imdata, 1/500)
269+
indsre = downsample(w, redata, 1/500)[3]
270+
indsim = downsample(w, imdata, 1/500)[3]
296271
inds = sort!(union(indsre, indsim))
297272
lines!(ax, redata[inds], imdata[inds], label=lab)
298273
else
@@ -349,7 +324,7 @@ function CSMakie.sigmaplot(systems::Union{LTISystem, AbstractVector{<:LTISystem}
349324
yscale = get_yscale_transform(),
350325
title = "Sigma Plot",
351326
xlabel = hz ? "Frequency [Hz]" : "Frequency [rad/s]",
352-
ylabel = "Singular Values $_PlotScaleStr[]")
327+
ylabel = "Singular Values $(ControlSystemsBase._PlotScaleStr)")
353328

354329
for (si, s) in enumerate(systems)
355330
sbal = balance ? balance_statespace(s)[1] : s
@@ -359,10 +334,9 @@ function CSMakie.sigmaplot(systems::Union{LTISystem, AbstractVector{<:LTISystem}
359334
sv = sv[:, [1, end]]
360335
end
361336

362-
if _PlotScale[] == "dB"
337+
if ControlSystemsBase._PlotScale == "dB"
363338
sv = 20*log10.(sv)
364339
end
365-
366340
# Use _to1series to efficiently plot multiple lines
367341
ws_series, sv_series = _to1series(ws, sv)
368342
lines!(ax, ws_series, sv_series, color=Cycled(si))
@@ -442,7 +416,7 @@ function CSMakie.marginplot(systems::Union{LTISystem, AbstractVector{<:LTISystem
442416
ax_mag = axes_mag[i, j]
443417
magdata = vec(bmag[i, j, :])
444418

445-
if _PlotScale[] == "dB"
419+
if ControlSystemsBase._PlotScale == "dB"
446420
magdata = 20*log10.(magdata)
447421
mag_margins = 20 .* log10.(1 ./ gm)
448422
oneLine = 0
@@ -454,7 +428,7 @@ function CSMakie.marginplot(systems::Union{LTISystem, AbstractVector{<:LTISystem
454428
# Plot magnitude response
455429
if adaptive
456430
wsi, magi, _ = downsample(ws, magdata,
457-
maximum(magdata) - minimum(magdata)/500)
431+
_span(magdata)/500)
458432
lines!(ax_mag, wsi, magi, label=_get_plotlabel(s, i, j))
459433
else
460434
lines!(ax_mag, ws, magdata, label=_get_plotlabel(s, i, j))
@@ -463,13 +437,12 @@ function CSMakie.marginplot(systems::Union{LTISystem, AbstractVector{<:LTISystem
463437
# Unity gain line
464438
hlines!(ax_mag, oneLine, color=:gray, linestyle=:dash, alpha=0.5)
465439

466-
# Gain margins
440+
# Gain margins - draw vertical lines from unity to margin value
467441
wgm_display = hz ? wgm ./ (2π) : wgm
468-
for k in 1:length(gm)
442+
for k in eachindex(wgm_display)
443+
# Draw vertical line from unity gain to the margin value
469444
lines!(ax_mag, [wgm_display[k], wgm_display[k]],
470-
[oneLine, mag_margins[k]], color=:red)
471-
scatter!(ax_mag, [wgm_display[k]], [mag_margins[k]],
472-
color=:red, markersize=8)
445+
[oneLine, mag_margins[k]], color=:red, linewidth=2)
473446
end
474447

475448
# Add title with margins info
@@ -485,21 +458,25 @@ function CSMakie.marginplot(systems::Union{LTISystem, AbstractVector{<:LTISystem
485458

486459
if adaptive
487460
wsp, phasep, _ = downsample(ws, phasedata,
488-
maximum(phasedata) - minimum(phasedata)/500)
461+
_span(phasedata)/500)
489462
lines!(ax_phase, wsp, phasep)
490463
else
491464
lines!(ax_phase, ws, phasedata)
492465
end
493466

494467
# Phase margin lines
495468
wpm_display = hz ? wpm ./ (2π) : wpm
469+
470+
# Draw horizontal lines at phase margin crossings
496471
for k in 1:length(pm)
497472
phase_line = fullPhase[k] - pm[k]
498473
hlines!(ax_phase, phase_line, color=:gray, linestyle=:dash, alpha=0.5)
474+
end
475+
476+
# Draw vertical lines showing the phase margins
477+
for k in 1:length(pm)
499478
lines!(ax_phase, [wpm_display[k], wpm_display[k]],
500-
[fullPhase[k], phase_line], color=:blue)
501-
scatter!(ax_phase, [wpm_display[k]], [fullPhase[k]],
502-
color=:blue, markersize=8)
479+
[fullPhase[k], fullPhase[k] - pm[k]], color=:blue, linewidth=2)
503480
end
504481

505482
# Add title with phase margins info

‎lib/ControlSystemsBase/src/CSMakie.jl‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -145,20 +145,8 @@ function leadlinkcurve(args...; kwargs...)
145145
error(MAKIE_NOT_LOADED_ERROR)
146146
end
147147

148-
"""
149-
setPlotScale(str)
150-
151-
Set the default scale of magnitude in `bodeplot` and `sigmaplot` for Makie plots.
152-
`str` should be either `"dB"` or `"log10"`. The default scale is `"log10"`.
153-
154-
Requires Makie to be loaded.
155-
"""
156-
function setPlotScale(str::AbstractString)
157-
error(MAKIE_NOT_LOADED_ERROR)
158-
end
159-
160148
# Also export the mutating versions that will be defined by the extension
161149
export bodeplot, nyquistplot, sigmaplot, marginplot, pzmap, pzmap!,
162-
nicholsplot, rgaplot, rlocusplot, leadlinkcurve, setPlotScale
150+
nicholsplot, rgaplot, rlocusplot, leadlinkcurve
163151

164152
end # module CSMakie

‎lib/ControlSystemsBase/test/test_makie_plots.jl‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
using ControlSystemsBase
22
using Test
33
using LinearAlgebra
4-
5-
# Check if Makie is available
6-
using Makie
7-
using CairoMakie # Use CairoMakie for non-interactive backend in tests
4+
using GLMakie
85

96
@testset "Makie Plot Tests" begin
107
# Create test systems
@@ -153,7 +150,7 @@ using CairoMakie # Use CairoMakie for non-interactive backend in tests
153150

154151
@testset "nicholsplot" begin
155152
@test_nowarn begin
156-
fig = nicholsCSMakie.plot(P)
153+
fig = CSMakie.nicholsplot(P)
157154
@test fig isa Makie.Figure
158155
end
159156
end

0 commit comments

Comments
 (0)