search

JuliaDynamics / RecurrenceAnalysis.jl

Recurrence Quantification Analysis in Julia
https://juliadynamics.github.io/RecurrenceAnalysis.jl/stable/
MIT License
45 stars 12 forks source link

1.0.0 release #20

Closed Datseris closed 5 years ago

Datseris commented 5 years ago

For stability reasons, a 1.0 release is necessary, so that we can start following SymVer as far as breaking changes are concerned. @heliosdrm is there anything missing or anything you would like to do before we do this?

heliosdrm commented 5 years ago

One question to decide on is what should be the default metric. The literature often advocates for the maximum norm, which is the simplest, and I followed that criterion. But it looks that the Euclidean norm is in this case more efficient because of BLAS optimization. Comparing with other software does not give a definitive answer: pyunicorn (Python) uses the maximum norm as default, whereas crqa (R) only use the Euclidean norm.

Besides, there are other features (not that much "breaking", but rather "extending" the functionality of the packages), which @pucicu suggested to me privately some time ago:

Besides, I have thought on adding RQA parameters related to whitespace vertical structures. And more ambitious - for the long term, allow paralellization - calculation of the recurrence plots and RQA parameters is quite fit for that feature.

Perhaps we could categorize those features as milestones for 1.0, and following minor/major versions? This is extending functionality

Datseris commented 5 years ago

Everything that extends the functionality is totally ok to be added after 1.0, please if possible open a dedicated issue for each functionality, maybe I can contribute as well :) I have dabbled a bit in parallelization and successfully achieved it for DynamicalBilliards

From what you've written the only concern is about the default norm, which we should think more. Yes the euclidean norm is much faster for Julia, but as you said most of the time the default arguments will be Datasets that will have to be converted... We haven't measured so far the conversion price. That should give us some perspective.

After we have finalized the default norm I say we take one final look at the existing API (which is in the new documentation) and then I vote to release 1.0. If something breaking comes about later we can go to version 2 and 3 and 4, that's okay. But I am a strong follower of the SymVer promises and besides, I think this package is well matured to be 1.0 :)

Datseris commented 5 years ago

(of course, we will have to release 1.0 for DelayEmbeddings as well :P )

heliosdrm commented 5 years ago

I don't think that conversion to Matrix will happen so often: only if the embedding dimension is equal or greater than 10, which is not usual.

I'll create the individual issues tomorrow or the day after.

Datseris commented 5 years ago

I don't think that conversion to Matrix will happen so often: only if the embedding dimension is equal or greater than 10, which is not usual.

Yeah, forgot about that, you are right! But coming back to the performance, I checked again the first post on https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/pull/19 and I got reminded that the speed difference for large N is pretty huge: Euclidean() totally wins over Chebyshev() and also Citiblock. In fact, Chebyshev is by far the slowest one of the three, which is a very strong argument for changing the default norm :D

heliosdrm commented 5 years ago

Yet another thing that I have in mind since the beginning, but I have not yet decided on it:

Currently recurrencematrix etc. create matrices that can be subsequently analyzed. But another possibility is to create an instance of a more sophisticated type, that besides containing the matrix, stores meta-information that may help to be more efficient in its analysis. E.g. recurrencematrix and jointrecurrencematrix always yield symmetric matrices, so if this is known, it wouldn't be necessary to call issymmetric later on in functions like diagonalhistogram.

The symmetry issue is only an example, for which there may be other workarounds - e.g. the Symmetric type, although it is not optimal for sparse Boolean matrices. There are other possibilities, but this needs some thought, and since I have not yet analyzed in depth the best structure for such a type, I thought that for the time being it was better just go for the simplest approach (no custom type, as it is now).

Datseris commented 5 years ago

I Fully support this idea!!! The idea of having dedicated types RecurrenceMatrix, CrossRecurrenceMatrix etc, all of which are subtypes of AbstractRecurrenceMatrix ! ! ! This has a lot of benefits:

  1. You choose the internal structure that RecurrenceMatrix wraps around, and even changing it later (for performance gains) won't be breaking.
  2. Multiple Dispatch enforces that the user is passing a recurrence matrix to e.g. laminarity, because the dispatch happens on ::AbstractRecurrenceMatrix (so far there is no guarantee that x is not a unfitting matrix)
  3. What you said about exploiting known symmetries
  4. Pretty printing (which I super love). So when the user sees x they will get info that it is a recurrence matrix, etc. This is really cool for front end users!
  5. Custom indexing methods (not necessary).

And many other things! I think we should go with that!

Datseris commented 5 years ago

@heliosdrm I am looking at the code that computes the recurrence matrix. I can see that everything is always propagated to the cross recurrence with x,x. But for the simple recurrence, only half the computations are necessary, as by definition the matrix is a symmetric matrix. Right?

Should we modify the recurrencematrix function to do only half the computations? Because now:

    x = x.data
    y = y.data
    rowvals = Vector{Int}()
    colvals = Vector{Int}()
    for j in 1:length(y)
        nzcol = 0
        for i in 1:length(x)
            @inbounds if evaluate(metric, x[i], y[j]) ≤ ε
                push!(rowvals,i)
                nzcol += 1
            end
        end
        append!(colvals, fill(j, (nzcol,)))
    end

or does this mess up with the way we have created the plotting function?

Datseris commented 5 years ago

@heliosdrm We have had tremendous progress so far and I feel like everything is looking to be very stable. We have concluded on the naming and nomenclature of all functions we use and https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/issues/45#issuecomment-455549094 concluded on the == operation.

I think we are in a state to release 1.0!

heliosdrm commented 5 years ago

I mostly agree. Nevertheless, since we cannot go back after tagging v1.0, I suggest we do a checklist of the possible issues that might imply breaking changes (not bug-fixing or non-breaking added features), and check that we are sure about the decisions that we have made - or think about it if have not made an explicit decision yet. In the cases of decisions already made, this list may come in handy for future reference.

This is the list of the issues I can think of, but please feel free to expand, we don't want to leave anything important!

Issues already discussed:

Potential issues not yet discussed (I'm going to open issues now for those that I really think that need discussion):

Default options not yet discussed

Let's go through the checklist, and when everything is clear, go for 1.0!!

Datseris commented 5 years ago

Awesome list, wow this post probably took a lot of time, good job!

I've checked the ones I have a final opinion, or that I think that it does not matter much:

Datseris commented 5 years ago

https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/pull/50 <- this is crazy

heliosdrm commented 5 years ago

I mostly agree again. I checked the "initial normalization" box: I also think that this must be done outside, and does not even need to be implemented here; it is just LinearAlgebra.normalize (or normalize!).

I'm reopening #45 for the default theiler, and I have just opened a couple of further issues (#48, #49) for the other unchecked questions.

About using UnicodePlots... I was going to write about it, but just saw that you opened an issue before :wink:

Datseris commented 5 years ago

On a side note; I will be doing a video tutorial on dynamical systems on 13th of Febr. on the official julia channel. I will of course show RecurrenceAnalysis as well.

So in general, we should try to have the 1.0 out before that, to ensure the video has the stable information. (In fact one of the reasons I am doing this video is because all other packages are in 1.0+)

For tracking purposes: https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/milestone/1

pucicu commented 5 years ago

very good! I will try to follow and if allowed, I could also advertise this on the RP channels (email list and twitter).

Am 19.01.2019 um 12:00 schrieb George Datseris notifications@github.com:

On a side note; I will be doing a video tutorial on dynamical systems on 13th of Febr. on the official julia channel. I will of course show RecurrenceAnalysis as well.

So in general, we should try to have the 1.0 out before that, to ensure the video has the stable information. (In fact one of the reasons I am doing this video is because all other packages are in 1.0+)

For tracking purposes: https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/milestone/1 https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/milestone/1 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/issues/20#issuecomment-455770281, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7WkJuY-v69vJ46gcW5GBfo_GbUJ4sPks5vEvrrgaJpZM4Zf_VB.

Datseris commented 5 years ago

very good! I will try to follow and if allowed, I could also advertise this on the RP channels (email list and twitter).

@pucicu , absolutely, please do so!!!

If you need an abstract you may copy/paste the one from the video description:

George Datseris from the Max Planck Institute for Dynamics and Self-Organization will give us an introduction to the dynamical systems ecosystem in Julia, which is encapsulated in the DynamicalSystems.jl Julia package. DynamicalSystems.jl is a general tool for modeling dynamical systems but is mostly focused for modeling chaos and nonlinear dynamics. It is made from the ground up with the principles of clarity, intuition and robustness.

Besides a collection of Julia packages, DynamicalSystems.jl is also a "library" in the literal sense. All offered functionality is well documented and accompanied with scientific descriptions of the algorithms as well as references to original papers, offering new users plenty of chances to learn new algorithms.

In this video we will be seeing an overview of all the features of DynamicalSystems.jl (now in version 1.0+) and go in depth into a few of them, as time allows. We will also be answering questions asked in the live chat.

heliosdrm commented 5 years ago

We're nearly there (see #52)!!

Since we have a few more days before the deadline of the milestone, I would like to look at the values of the tests (not only check that they are passing). Is that ok?

By the way, Julia 1.1 is also near, but CI for nightly is disabled. Shouldn't we test for that too?

heliosdrm commented 5 years ago

There is a nice property derived from the combination of #51 and #52: the previous behavior, i.e. default theiler=0 and always divide by full size of the matrix for RR regardless of the Theiler window, as done e.g. by crqa, remains for CrossRecurrenceMatrix, so if one wants to obtain comparable results the only that has to do is always use that type, e.g. CrossRecurrenceMatrix(x,x,e) instead of RecurrenceMatrix(x,e)

Datseris commented 5 years ago

Since we have a few more days before the deadline of the milestone, I would like to look at the values of the tests (not only check that they are passing). Is that ok?

Sure, there is plenty of time, no stress!

By the way, Julia 1.1 is also near, but CI for nightly is disabled. Shouldn't we test for that too?

We could re-enable it. I removed it because we were developing very fast and it wasn't worth it to wait for all those extra tests. (Also, I've read the 1.1 changelog, nothing there affects us)

heliosdrm commented 5 years ago

Ok, so I think that it's ready to tag a pre-release (1.0a or 1.0-rc), so that we can test It;

Datseris commented 5 years ago

Done: https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/releases/tag/v1.0.0-alpha

edit: you can't install this with the package manager though: https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/issues/53

heliosdrm commented 5 years ago

I have made some tests, comparing with manual calculations, published examples and the output of other packages. The results are identical (differences are smaller than 1e-6), and I only detected a bug in TREND, which unfortunately I could not compare with other software (solved in #54). From my side I have no further tests to do. Perhaps only revise the documentation.

For what might be worth, I also updated the information of the old wiki where I had compared this package with other RQA software (I have not yet tested the current version it in a Windows machine, but performance was similar the last time I tested it): https://github.com/JuliaDynamics/RecurrenceAnalysis.jl/wiki/Comparison-of-software-packages-for-RQA

(Later on we could integrate some of this information in the documentation of DynamicalSystems.jl, or in some paper with a bit of more serious work.)

We have improved performance a lot! It is almost twice as fast as it was in version 0.3.0, and it approaches very closely the speed of pyunicorn, but with the advantage of being pure Julia, without "glue code".

(The next challenge is comparing with PyRQA, which takes advantage of GPU). :smiley:

heliosdrm commented 5 years ago

One more thing: we are still supporting the deprecated older function names. Should this be off in v1.0.0?

Datseris commented 5 years ago

Yes.

heliosdrm commented 5 years ago

Well, I don't have anything else to revise that blocks the release of 1.0.0. I have a few things in store, but only new features and non-breaking improvements that can wait. @Datseris : it is up to you now.

Datseris commented 5 years ago

I just saw #59 . Let's resolve this and then we can release! :)

Datseris commented 5 years ago

Alright @heliosdrm we did a very good job here. I am just notfying that I'll release 1.0 tomorrow. So you have today to stop me if need be!

heliosdrm commented 5 years ago

I had nothing else breaking to add. But noted your doubt in #64 about TREND and submitted #68 to address it - if you think that such change is advisable.

heliosdrm commented 5 years ago

Oh, yes: and the tests should also check against Julia 1.1, I think.