Generic type for basins in `basins_of_attraction`

[KalelR]

A solution to the problem of arbitrary labels in the featuring supervised method

[KalelR]

Sorry if I messed something up with the commits here, don't know why it's ahead of main...

This change initializes basins with Anytype, which then allows it to be filled with the generic labels for the supervised method. In all other cases however it will still be returned as an Array of Any, when it could be the more specific Int type. A solution to this would be the line if all(typeof.(labels) .<: Int) basins = convert(Array{Int}, basins) end

Is that ok?

[Datseris]

@KalelR I suspect the reason the git is a mess is because you are not branching off main. you are branching off other branches that are later merged into main. So git doesn't register the commit differences. After a PR is merged, best practice for you, assuming your PRs come from branches of the origin, and not from your own fork: (1) checkout main locally, (2) sync main with the remote main, that has your PR merged in, (3) start a new branch after you have updated your local main.

[Datseris]

Also, by trying to update the this PR to main, I removed your commit in the test file that removed the sort! function, sorry about that! Please do go ahead and restore this change!

[Datseris]

@KalelR okay, I resetted after a nice sleep and rest. My idea to allow generic types was stupid. It takes much more effort from our side. From a user perspective, they can always define a new dictionary mapping their labels to integers. Our code should always operate on integers like before. Could you please update this pr to make sure our code uses only integers? I am really sorry to make you do double work, but I thought this through and what we are trying to do doesn't really align with the principles of DynamicalSystems.jl, where things should really only be done one way and the user should always convert to this one way.

[KalelR]

@KalelR okay, I resetted after a nice sleep and rest. My idea to allow generic types was stupid. It takes much more effort from our side. From a user perspective, they can always define a new dictionary mapping their labels to integers. Our code should always operate on integers like before. Could you please update this pr to make sure our code uses only integers? I am really sorry to make you do double work, but I thought this through and what we are trying to do doesn't really align with the principles of DynamicalSystems.jl, where things should really only be done one way and the user should always convert to this one way.

That's alright. It would have been a nice feature but unfortunately it is not useful enough for the amount of effort needed. I now resetted the code for basins_of_attraction so that basins is of Int type. I am keeping the previous addition to cluster_features_distances, with the correct replacing of labels. With this, basins_fractions still works with arbitrary labels in the templates (in the supervised method). And basins_of_attractions will also return the correct labels for any Int labels the user passes. Should we state in the documentation that the labels must be of Int type, to avoid confusion? They only really need to be for basins_of_attraction.

[Datseris]

Yes, state in the doc that labels must be of type Int and that -1 is a reserved label for invalid trajectories (that we don't know where they converge, or they escape to infinity)

[Datseris]

Also please add a default value for clustering that was like our previous default keywords for clustering. I think you just need to write

AttractorsViaFeaturizing(ds::GeneralizedDynamicalSystem, featurizer::Function,
    cluster_config::ClusteringConfig = ClusteringConfig()

in the function definition.