Skip to content

Qualify parameters in the LowLevelParticleFilters methods, add timeevol to FRD - #201

Merged
baggepinnen merged 4 commits into
masterfrom
fix-llpf-parameters
Oct 1, 2026
Merged

baggepinnen merged 4 commits into
masterfrom
fix-llpf-parameters

Conversation

@baggepinnen

@baggepinnen baggepinnen commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Summary

forward_trajectory(kf, d::AbstractIdData), smooth(kf, d) and smooth(kf, M, d) used parameters(kf) as the default value of p. LowLevelParticleFilters does not export parameters, so calls without p raised UndefVarError: parameters not defined in ControlSystemIdentification. The default is now LowLevelParticleFilters.parameters(kf).

smooth(kf::KalmanFilter, d) additionally required a change in LowLevelParticleFilters: its method smooth(kf::KalmanFilter, args...) was more specific than smooth(kf::AbstractFilter, d::AbstractIdData) and failed for AbstractIdData. LowLevelParticleFilters.jl#290 restricts that method to vector data (version 3.33.1). The new smooth test is therefore only run with LowLevelParticleFilters ≥ 3.33.1; the forward_trajectory test runs unconditionally. The compat bound is unchanged.

FRD and ControlSystemsBase 1.22

CI on this PR exposed an unrelated failure with ControlSystemsBase 1.22: nyquist(frd::FRD) calls freqresp(sys, w; balance). freqresp(::FRD, w) accepted no keyword arguments, so the call was dispatched to the generic freqresp(::LTISystem, w; balance), which failed with FieldError: type FRD has no field timeevol, and, once that was resolved, with a MethodError for numeric_type(::FRD).

  • FRD now has the property timeevol, which returns Continuous(), consistent with its supertype LTISystem{Continuous}. iscontinuous(frd) therefore works. propertynames lists timeevol, nu and ny as well.
  • freqresp(::FRD, w; kwargs...) accepts and ignores keyword arguments such as balance.

Other changes

  • The absolute tolerance of the frequency-response comparison in test_basis_functions.jl is increased from 1e-10 to 2e-9. With ControlSystemsBase 1.22, the difference for the Kautz basis exceeded 1e-10 for about 70% of the random coefficient vectors (largest observed value 6.7e-10 over 300 draws; relative difference at most 2e-13).
  • Version bump to 2.12.1.

Found during LowLevelParticleFilters.jl#288.

Tests

  • test_subspace.jl checks forward_trajectory(kf, d) and smooth(kf, d) for a filter obtained from subspaceid.
  • test_frd.jl checks iscontinuous(frd) and freqresp(frd, w; balance = false); it passes locally with ControlSystemsBase 1.22.0 (56 passed).
  • test_subspace.jl passes locally on Julia 1.13 against the branch of LowLevelParticleFilters.jl#290 (2690 passed). The rest of the test suite was not run locally.

Host: demeter2, Claude Code session a4b42284-297c-4d79-b33c-7149cef31ba8

🤖 Generated with Claude Code

…actIdData

`parameters` is not exported by LowLevelParticleFilters, so `forward_trajectory(kf, d::AbstractIdData)` and `smooth(kf, d)` raised `UndefVarError: parameters` when called without the parameter argument.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…resp(::FRD)

ControlSystemsBase 1.22 calls `freqresp(sys, w; balance)` from `nyquist`. Since `freqresp(::FRD, w)` accepted no keyword arguments, the call was dispatched to the generic `freqresp(::LTISystem, w; balance)`, which failed in `timeevol(::FRD)` and, after that, in `numeric_type(::FRD)`. `FRD` now has the property `timeevol = Continuous()`, consistent with its supertype `LTISystem{Continuous}`, and `freqresp(::FRD, w; kwargs...)` ignores the keyword arguments.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@baggepinnen baggepinnen changed the title Qualify parameters in the LowLevelParticleFilters methods for AbstractIdData Qualify parameters in the LowLevelParticleFilters methods, add timeevol to FRD Oct 1, 2026
baggepinnen and others added 2 commits October 1, 2026 13:29
For the Kautz basis with poles up to 1e3 rad/s, the absolute difference between the two frequency-response computations exceeded 1e-10 for about 70% of the random coefficient vectors with ControlSystemsBase 1.22, while the relative difference was at most 2e-13.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ion to 2.12.1

The largest difference observed for the Kautz basis over 300 random coefficient vectors was 6.7e-10.

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

codecov Bot commented Oct 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.50%. Comparing base (bfc0f7c) to head (6d49a13).

Files with missing lines Patch % Lines
src/subspace.jl 33.33% 2 Missing ⚠️
src/frd.jl 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #201      +/-   ##
==========================================
+ Coverage   89.49%   89.50%   +0.01%     
==========================================
  Files          15       15              
  Lines        2722     2726       +4     
==========================================
+ Hits         2436     2440       +4     
  Misses        286      286              
Flag Coverage Δ
unittests 89.50% <50.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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 6c00501 into master Oct 1, 2026
3 of 4 checks passed
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.

1 participant