search

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
53 stars 12 forks source link

Erroneous conversion between logarithm bases for `DifferentialEntropyEstimators` #256

Closed kahaaga closed 1 year ago

kahaaga commented 1 year ago

Describe the bug

The conversion between logarithm bases for the differential entropy estimators is not correct. The conversion formula is 

\log_a(h) = \dfrac{\log_c(h)}{\log_c(a)}

Currently, we're doing (see denominator)

\log_a(h) = \dfrac{\log_c(h)}{\log_a(c)}

I remember checking the correctness of the conversion back in the days, but sometime during development of v2, I must have flipped the sign, and due to copy-pasting, the error ended up encompassing all the differential entropy estimators in a consistent manner. The same error also persisted through the doc examples and the tests. I discovered this error while debugging some strange results for the mutual information in CausalityTools.jl for differential entropy estimators.

The tests still pass because the error is present both in the estimator code, and in the values we check against.

To avoid this error, there an obvious fix: define an (tested) function that takes care of the conversion, and use it everywhere where it applies. I have a fix ready, and will submit a PR in a moment.

Package versions

This error occurs for all 2.X versions.

Datseris commented 1 year ago

i think this mistake was mine, but thanks for the fix :D logarithms are hard man!!!

kahaaga commented 1 year ago

i think this mistake was mine, but thanks for the fix :D logarithms are hard man!!!

I might as well have messed it up too. I have to re-do the conversion calculation every time, but probably just got too confident at some point at started just going for it :D I'm fairly certain this is correct now, but lets see... ☠️