Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The function lt in OrdinalPatternEncoding isn't actually used #378

Open
1 task
kahaaga opened this issue Jan 13, 2024 · 1 comment · May be fixed by #415
Open
1 task

The function lt in OrdinalPatternEncoding isn't actually used #378

kahaaga opened this issue Jan 13, 2024 · 1 comment · May be fixed by #415
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@kahaaga
Copy link
Member

kahaaga commented Jan 13, 2024

The OrdinalPatterns documentation string promises that the user can provide a comparator function (defaults to lt = isless) to determine how two elements of a state are deemed to be "equal" (useful to prevent bias when there are tied values).

This function lt gets stored in the encoding::OrdinalPatternEncoding field of the OrdinalPatterns struct. However, the function isn't actually used in encode. We did use it before, but after we transitioned to the formal encode/decode interface, we forgot to pass the argument on to the underlying sortperm! call.

The fix is easy. Here's the source code:

function encode(encoding::OrdinalPatternEncoding{m}, χ::AbstractVector) where {m}
    if m != length(χ)
        throw(ArgumentError("Permutation order and length of input must match!"))
    end
    perm = sortperm!(encoding.perm, χ)
    return permutation_to_integer(perm)
end

TODO:

  • Pass on lt to sortperm! when computing the permutation pattern.
@kahaaga kahaaga added bug Something isn't working good first issue Good for newcomers labels Jan 13, 2024
@Datseris
Copy link
Member

We should add a simple test for this case, like for the permutation entropy of ones(1000); one would give 0 (the one with isless) while the other one would give maximum (with the random comparison).

Datseris added a commit that referenced this issue Jun 10, 2024
@Datseris Datseris linked a pull request Jun 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants