Typo: Missing =
Closed this issue · 2 comments
Just browsing the package, looks awesome!
I was looking at the actual implementation of GaussianMI
and I think there's a typo here:
https://github.com/JuliaDynamics/CausalityTools.jl/blob/f1bc2b768778f2670587c87022f5e314e85eab0c/src/methods/infomeasures/mutualinfo/estimators/GaussianMI.jl#L70-L74
The last line should probably be
mi = -0.5 * sum(log(σᵢ) for σᵢ in σ)
Also there's a small typo in the doc here:
https://github.com/JuliaDynamics/CausalityTools.jl/blob/f1bc2b768778f2670587c87022f5e314e85eab0c/src/methods/infomeasures/mutualinfo/estimators/GaussianMI.jl#L49-L51
Should be \sum{n=1}
-> \sum_{n=1}
. It shows up weird in the docs.
I can make a PR in a bit when I'm back at my workstation.
Well spotted, @RomeoV! You're right about both typos. If you could contribute a PR, that would be excellent 👍
If you can come up with a good test for the normalized GaussianMI
case, that would also be awesome to have (the current test suite didn't pick up on the typo, so a test for it has to be missing). If you don't want to do so in your PR, no problem - I'll just open an issue for it and deal with it later.
I'm not super well versed in the theory, but I can try to come up with a test that compares, say, the MI(normalize=true)
of unnormalized data with the MI(normalize=false)
of the already normalized data, and then also compare those to the theoretical result for some random samples (with fixed seed). Does that seem reasonable?
EDIT: Moved the discussion to a new issue.