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

Probability estimators shouldn't enforce knowledeg of input #229

Closed Datseris closed 1 year ago

Datseris commented 1 year ago

Closes https://github.com/JuliaDynamics/CausalityTools.jl/issues/183

@kahaaga can you please test your mutualinfo implementation with ValueHistogram and NaiveKernel with this PR?

there are still some estimators I have to change but let's first test it.

kahaaga commented 1 year ago

an you please test your mutualinfo implementation with ValueHistogram and NaiveKernel with this PR?

Yes, will do.

@Datseris FYI: There are some errors in this PR that prevents compilation, so I have to fix it. Remember to fetch the changes I make before working on it any further.

kahaaga commented 1 year ago

Ok, I've fixed the PR to the best of my ability, given the first glance. All tests now pass, and all doc examples now work again. I also quality-checked quickly the changed documentation again.

EDIT: The tests pass locally, but don't pass CI. The error message is

ERROR: LoadError: SystemError: opening file "/home/runner/work/Entropies.jl/Entropies.jl/src/entropies_definitions/Renyi.jl": No such file or directory

which makes no sense, because the file definitely exists.

Take-away

Everything now works as expected upstream (so https://github.com/JuliaDynamics/CausalityTools.jl/issues/183 is no longer an issue, and I'm closing that).

I might have messed up something in the probabilities.jl file, though, so please have a final look at the generic probabilties API functions before merging.

Further changes

I took the liberty to fix #228 in this PR too and implement Shannon as its own type, because that simplifies code upstream massively too. The code for all the DifferentialEntropyEstimators is now much cleaner, because we don't need to check if the input entropy is a Shannon entropy (it is now enforced by the definition).

Datseris commented 1 year ago

ok i finish this after lunch and merge.

Datseris commented 1 year ago

you're a windows user and windows system is not case sensitive. renaming the files to capitalized has no impact on windows and therefore the git system.

codecov[bot] commented 1 year ago

Codecov Report

Merging #229 (a4790e6) into main (8120183) will decrease coverage by 0.14%. The diff coverage is 84.90%.

:exclamation: Current head a4790e6 differs from pull request most recent head b94b842. Consider uploading reports for the commit b94b842 to get more accurate results

@@            Coverage Diff             @@
##             main     #229      +/-   ##
==========================================
- Coverage   84.77%   84.62%   -0.15%     
==========================================
  Files          45       46       +1     
  Lines        1136     1132       -4     
==========================================
- Hits          963      958       -5     
- Misses        173      174       +1     
Impacted Files Coverage Δ
src/entropies_definitions/renyi.jl 93.33% <ø> (-0.79%) :arrow_down:
...rs/spatial/spatial_dispersion/SpatialDispersion.jl 85.36% <ø> (ø)
..._estimators/transfer_operator/transfer_operator.jl 75.18% <ø> (ø)
src/probabilities_estimators/wavelet_overlap.jl 82.60% <33.33%> (ø)
src/probabilities_estimators/power_spectrum.jl 69.23% <40.00%> (+4.94%) :arrow_up:
...rc/encoding_implementations/rectangular_binning.jl 87.69% <75.00%> (-0.84%) :arrow_down:
src/probabilities.jl 78.57% <75.00%> (-2.20%) :arrow_down:
src/probabilities_estimators/value_histogram.jl 91.66% <87.50%> (-2.09%) :arrow_down:
src/complexity_measures/missing_dispersion.jl 100.00% <100.00%> (ø)
src/convenience.jl 94.11% <100.00%> (ø)
... and 15 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

kahaaga commented 1 year ago

you're a windows user and windows system is not case sensitive. renaming the files to capitalized has no impact on windows and therefore the git system.

Weird. All other files in the package with capitalised filenames have been recognized before, so I don't understand why this particular Renyi.jl wouldn't be recognized too.

I'm using MacOS, btw.

Datseris commented 1 year ago

Weird. All other files in the package with capitalised filenames have been recognized before, so I don't understand why this particular Renyi.jl wouldn't be recognized too.

None of them were. You only say Renyi because it is the first one called.

kahaaga commented 1 year ago

But Shannon.jl was the first file, not Renyi.jl. So then the error should have occurred when reading Shannon.jl.

Also, most files in src/entropy_definitions have capitalized names, which are read without any issue. Take-away: I'm still confused.

Datseris commented 1 year ago

you created Shannon file, not renamed it.so it was registered with capital name. You rejnamed the other files to capitalized but this change wasn't propagated by git. here is the commit you did that. https://github.com/JuliaDynamics/Entropies.jl/pull/229/commits/172853fe77ed1f27d50d1c97f68b79546dadefe5

you see no file rename is communicated.

kahaaga commented 1 year ago

Ah, I see. Thanks for the explanation.