Closed rusandris closed 9 months ago
Thanks for opening this, @rusandris. The problem is here:
function codify(est::OrdinalOutcomeSpace{m}, x) where m
if x isa AbstractVector
dataset = embed(x, m, est.τ)
else
dataset = x
end
m != dimension(dataset) && throw(ArgumentError(
"Order of ordinal patterns and dimension of `StateSpaceSet` must match!"
))
πs = zeros(Int, length(dataset))
@inbounds for (i, χ) in enumerate(dataset)
πs[i] = encode(est.encoding, χ)
end
return πs
end
As per the docstring of OrdinalPatterns
, we have:
AbstractVector
, then it is assumed that embedding should be done.x
is a StateSpaceSet
, then codify
treats it as is: assuming that the user has pre-embedded their data. This is essentially what you've done here by generating data using trajectory
, which always returns a StateSpaceSet
(although in the case of 1D systems, it is just the identity embedding).This behavior will not change.
However, we could perhaps add an extra error message for when the input is a state-space set whose dimension doesn't match m
. Perhaps we could add a method like:
function codify(est::OrdinalOutcomeSpace{m}, x::StateSpaceSet{D}) where {m, D}
if (m != D)
throw(ArgumentError("A `D`-dimensional `StateSpaceSet` was given as input, but the outcome space has ordinal pattern length `m = $m`. These must match."))
end
end
What do you think, @rusandris? The precise wording of the error message can be discussed, of course. We could potentially also add a specific method for StateSpaceSet{1}
with a gentle reminder to the user that they have to convert their 1D state space set to a vector, if they've forgotten (EDIT: if they want the data to be embedded).
We could potentially also add a specific method for
StateSpaceSet{1}
Yep, seems reasonable. The method could look like something like this:
function codify(est::OrdinalOutcomeSpace{m}, x::StateSpaceSet{1})
throw(ArgumentError("A `StateSpaceSet` input is assumed to be already embedded. Convert your univariate time series to a subtype of `AbstractVector` to codify with ordinal patterns!"))
end
Yep, seems reasonable. The method could look like something like this:
Looks good to me. Feel like doing the PR, @rusandris?
We should probably also add a comment that ordinal pattern outcome spaces are only defined for m >= 2
, so they will never work with one-dimensional state space sets anyways (or something along those lines). I'm not sure if this is immediately obvious to a user that tries what you did and gets this warning.
We should probably also add a comment that ordinal pattern outcome spaces are only defined for m >= 2
Added this to the PR #359
Hi! When using
codify
withOrdinalPatterns
on a 1DStateSpaceSet
like thisthe following error is raised:
ERROR: ArgumentError: Order of ordinal patterns and dimension of `StateSpaceSet` must match!
which makes it hard to figure out the the actual problem is a method error. For univariate data, one should convert to a vector-like container:
vec(Matrix(orbit))
which seems cumbersome to me.Possible remedies:
codify
to accept bothStateSpaceSet
andAbstractVector
regardless of dimension (type union?)StateSpaceSet
Let me know what you think of these suggestions, I'm happy to help with PRs.