JuliaDynamics / ComplexityMeasures.jl

Estimators for probabilities, entropies, and other complexity measures derived from data in the context of nonlinear dynamics and complex systems
MIT License
56 stars 14 forks source link

Warn when `binning.precise == false` for `TransferOperator` #328

Closed kahaaga closed 9 months ago

kahaaga commented 11 months ago

I was getting weird errors in CausalityTools when using the TransferOperator for estimating transfer entropy. It turns out the reason is a feature I forgot we introduced: the option to turn on/off the precise option for RectangularBinning/FixedRectangularBinning. The default is precise == false when constructing instances of either, because that is more performant. However, in some cases for TransferOperator, this causes some points to be encoded as -1 even though they are inside the binning. In turns, this causes some hard-to-debug bugs.

After this PR, when any relevant function calls the internal transferoperator function with precise == false, a warning will be thrown. There may be cases where -1-encodings are not critical (i.e. entropy estimation here), but in CausalityTools.jl I explicitly need to decode the encoded bins again to get their bin indices, which is not possible when the index is -1.

kahaaga commented 11 months ago

Or maybe warn_precise should be an optional keyword argument to the TransferOperator constructor?

codecov[bot] commented 11 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (70da737) 88.03% compared to head (039f66f) 88.17%. Report is 8 commits behind head on main.

Files Patch % Lines
...come_spaces/transfer_operator/transfer_operator.jl 55.55% 4 Missing :warning:
src/outcome_spaces/ordinal_patterns.jl 92.85% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #328 +/- ## ========================================== + Coverage 88.03% 88.17% +0.13% ========================================== Files 78 78 Lines 2031 2046 +15 ========================================== + Hits 1788 1804 +16 + Misses 243 242 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Datseris commented 11 months ago

Or maybe warn_precise should be an optional keyword argument to the TransferOperator constructor?

This is the way to go I feel. Make this true by default.

Datseris commented 11 months ago

Or maybe warn_precise should be an optional keyword argument to the TransferOperator constructor?

This is the way to go I feel. Make this true by default.

rusandris commented 10 months ago

This behaviour might be also related to this issue:

using DynamicalSystems
henon_rule(x, p, n) = SVector{2}(1.0 - p[1]*x[1]^2 + x[2], p[2]*x[1])
henon = DeterministicIteratedMap(henon_rule, zeros(2), [1.4, 0.3])
orbit, t = trajectory(henon, 20_0000; Ttr = 500)

using ComplexityMeasures
grid_size = 20
binning = RectangularBinning(grid_size)
probabilities(TransferOperator(binning),orbit)

This raises a weird error: ERROR: LoadError: ArgumentError: Cannot decode integer -1: out of bounds of underlying binning.

kahaaga commented 9 months ago

I think this is also causing #338

EDIT: as pointed out in the comment above