baggepinnen / ControlSystemIdentification.jl

System Identification toolbox, compatible with ControlSystems.jl
https://baggepinnen.github.io/ControlSystemIdentification.jl/dev
MIT License
132 stars 13 forks source link

Documentation inconsistency and related question #158

Closed CasBex closed 3 weeks ago

CasBex commented 3 weeks ago

Hi, let me preface this by saying that I like this package and using it has generally been great.

The PEM section of the state-space identification documentation says the following:

A trade-off between prediction and simulation performance can be achieved by optimizing the h-step prediction error. The default is h=1 which corresponds to the standard prediction error. This can be changed using the keyword argument h. A large value of h will make the optimization problem computationally expensive to solve.

However, the newpem function doesn't actually take h as an argument according to the documentation string.

Either I've not understood this correctly or one of them is wrong.

Related question: is it currently possible to use newpem with multi-step prediction error?

baggepinnen commented 3 weeks ago

Thanks :)

h appears to be missing from the docstring, the function does take h https://github.com/baggepinnen/ControlSystemIdentification.jl/blob/master/src/pem.jl#L164

baggepinnen commented 3 weeks ago

close by https://github.com/baggepinnen/ControlSystemIdentification.jl/commit/56984f07ac48e53146059d505d3e00d0cadd9011