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

move to SSSets v1 and DynamicalSystemsBase v3 #249

Closed Datseris closed 1 year ago

Datseris commented 1 year ago

Move to StateSpaceSets.jl v1. Only mass renames, nothing else changed.

kahaaga commented 1 year ago

The tests fail because StateSpaceSet doesn't have an implementation for Base.IteratorSize.

Datseris commented 1 year ago

(in a meeting, if you can do PR otherwise i do later)

kahaaga commented 1 year ago

I don't have write access to StateSpaceSets, so it'll probably be quicker if you do it later

kahaaga commented 1 year ago

The latest version of Combinatorics, both indicated by the github repo and on JuliaHub is v1.0.2, so indicating 1.0.3 won't work

Datseris commented 1 year ago

I am so confused. How do we use datasets in this package that I have always errors that the StateSpaceSets.jl test suite doesn't get? I don't get it. What is this special syntax that we use...?

kahaaga commented 1 year ago

I am so confused. How do we use datasets in this package that I have always errors that the StateSpaceSets.jl test suite doesn't get? I don't get it. What is this special syntax that we use...?

You didn't change anything but the name when renaming?

I can check out the branch and see if I can figure out what's going on, but won't have time to do it before tomorrow

Datseris commented 1 year ago

I've finally found and fixed the problem. For fucks sake that was so sloppy (from my part)

Datseris commented 1 year ago

(locally, tests pass except the transfer operator tests that also fail on main. will push when DynamicalSystemsBase 3.0 is also tagged, as I updated that here as well)

Datseris commented 1 year ago

I just submitted DynamicalSystemsBase v3 for tag, so I need to move towards the release of DynamicalSystems.jl v3.0. I will try to solve the TO tests here but if they do not get solved, I will probably add an error message to the creation to TO saying that "Currently doesn't work" and proceed with tagging.

(I need this package to have v1 of StateSpaceSets.jl)

Datseris commented 1 year ago

Unfortunately I am not familiar enough with the internals of TO to solve this. The question is: were the tests of TO ever passing when moving to v2.0?

Datseris commented 1 year ago

The problem is here:

# The L points visits a total of L bins, which are the following bins (referenced
    # here as cartesian coordinates, not absolute bins):
    visited_bins = map(pᵢ -> encode(encoding, pᵢ), pts)

visited_bins contains -1 as entry.

kahaaga commented 1 year ago

Unfortunately I am not familiar enough with the internals of TO to solve this. The problem is here:

I also can't do this from the top of my head. I need to sit down and understand why the -1 occurs.

The question is: were the tests of TO ever passing when moving to v2.0?

They were working for what was supposed to be 2.0 at some point, but stopping working after the binning refactoring.

For now, I'm okay with just adding a "not working" at the moment. We can test this explicitly by a @test_throws WhateverError probabilities(TransferOperator(), x).

Because of this error, the TransferOperator estimator isn't implemented for CausalityTools yet either, so should be okay to just error it for now.

Datseris commented 1 year ago

I solved the problem!!!

It is because we were not using the precise variant of the encodings.

Datseris commented 1 year ago

Should we make the precise = true the default? It is 4x slowdown though.

Datseris commented 1 year ago

I've changed the constructor of RectangularBinning(::Int) to use 2 times next float. THat was the only binning that was erroring. Now we can leave things as is. I'll merge and tag now.

(after tests pass)

kahaaga commented 1 year ago

Should we make the precise = true the default? It is 4x slowdown though.

Yes, do that! And perhaps open an issue, so we can remember to optimize this too.

Could you just do a quick test that you get approximately the same entropy for the ValueHistogram and TransferOperator with the same binning, so we're sure nothing is broken?

b = FixedRectangularBinning(0:0.2:1.0)
x = Dataset(rand(100000, 3))
abs(entropy(TransferOperator(b), x) - entropy(ValueHistogram(b), x) ) < 0.1 # or something like that
Datseris commented 1 year ago

okay I am also updating eveyrthing to DYnamicalSystemsBase 3.0 in thsi PR

Datseris commented 1 year ago

oh god fingers crossed!

Datseris commented 1 year ago

I've tested everything locally, and built docs locally, all is ok.

kahaaga commented 1 year ago

I've tested everything locally, and built docs locally, all is ok.

Perfect. If the tests and doc build are successfull, then go ahead and merge!

codecov[bot] commented 1 year ago

Codecov Report

Merging #249 (f2eb069) into main (e2cf0c8) will increase coverage by 0.03%. The diff coverage is 86.20%.

@@            Coverage Diff             @@
##             main     #249      +/-   ##
==========================================
+ Coverage   81.03%   81.06%   +0.03%     
==========================================
  Files          49       49              
  Lines        1234     1236       +2     
==========================================
+ Hits         1000     1002       +2     
  Misses        234      234              
Impacted Files Coverage Δ
src/encoding_implementations/ordinal_pattern.jl 86.79% <ø> (ø)
src/multiscale.jl 0.00% <0.00%> (ø)
src/probabilities.jl 89.28% <ø> (ø)
src/probabilities_estimators/value_histogram.jl 87.50% <ø> (ø)
..._estimators/transfer_operator/transfer_operator.jl 75.20% <66.66%> (ø)
...rc/encoding_implementations/rectangular_binning.jl 90.24% <100.00%> (+0.24%) :arrow_up:
src/entropies_estimators/nearest_neighbors/Gao.jl 100.00% <100.00%> (ø)
...rc/entropies_estimators/nearest_neighbors/Goria.jl 100.00% <100.00%> (ø)
...estimators/nearest_neighbors/KozachenkoLeonenko.jl 100.00% <100.00%> (ø)
.../entropies_estimators/nearest_neighbors/Kraskov.jl 100.00% <100.00%> (ø)
... and 7 more

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