Skip to content

Conversation

baggepinnen
Copy link
Owner

I'd like to include some actual fitting of a first-principles model to drive home the point well enough to include

I'd like to include some actual fitting of a first-principles model to drive home the point well enough to include
@baggepinnen baggepinnen requested a review from Copilot May 27, 2025 09:29
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a new case study to the documentation demonstrating the impact of a single-sample delay on model estimation using a double-mass system example. Key changes include the introduction of a detailed step-by-step example, the generation of simulation data, and the comparison between ARX and subspace identification methods.


If we specify that we have a delay (1 sample delay implies causality, 2 samples delay implies causality + one sample pure delay), we indeed get what we would expect:
```@example DELAY
model_arx_2 = arx(d, 3, 3, inputdelay=2)
Copy link

Copilot AI May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usage of 'inputdelay=2' for a single-sample delay may be confusing; please add clarification in the text to explain the relationship between the specified delay value and the system delay representation.

Copilot uses AI. Check for mistakes.

model_arx = arx(d, 3, 3)
bodeplot!(bp, model_arx, lab="ARX model")
```
As we can see, the fit is terrible! The least-squares estimation that underpins the [`arx`](@ref) estimator tries to find a model that relates the last three outputs and inputs to the current output, but this is not possible due to the extra delay, in this case the result is catastrophic due to the "shortsightedness" of the [`arx`](@ref) function, it only looks at the immediate past and future, i.e., high-frequency properties.
Copy link

Copilot AI May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider revising the informal term 'catastrophic' to a more neutral expression to maintain an objective tone in the documentation.

Copilot uses AI. Check for mistakes.

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