Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store UPOs in a set & test possibly wrong #334

Merged
merged 4 commits into from
Jun 11, 2024

Conversation

JonasKoziorek
Copy link
Contributor

Previously UPOs in periodicorbits and davidchacklai algorithms were stored in a RBTree. This worked OK, however I noticed that sometimes duplicates occur even though they shouldn't. I could not trace down the bug, it seemed that rarely the equality == in the RBTree implementation just didn't work. I was testing and benchmarking other alternatives and my best solution is to use Set with linear search for duplicates. It's not ideal but it benchmarks better than before both in time and number of allocations. Furthermore it's more readable and clean than the previous code with VectorWithEpsRadius.

Also, I noticed that in this test file:

using ChaosTools
using Test
test_value = (val, vmin, vmax) -> @test vmin <= val <= vmax
@testset "standard map" begin
# Known fixed point locations from publication:
o2x = [0.0
0.0
3.1416

the publication fixed points might be incomplete. I found 2 more for period 2 and 3 more for period 3.

DL
SD
publication

The test can stay like this for now, but I think that it is needed to find some analytically know UPOs in literature and introduce some systematic testing for all the UPO algorithms.

@Datseris
Copy link
Member

Datseris commented Jun 8, 2024

Have a look at the "extra" points you found in more detail: They are 2-pi duplicates due to the symmetry of the map. They aren't more, its all about whether you make your grid go up to 2-pi or stop before 2-pi so that you don't get the duplicates. The exception here is the fixed point at (pi, 0), which must be a fixed point of all orders because it is fixed point of order 1. I don't know why schlemmer diakonos algorithm doesn't find that one... But sure, finding analytic UPOs would be nice, but it is unlikely, besides some super contrived cases where I am not sure that we would care about (that means: testing only these contrived cases would be a mistake; testing a chaotic map is really what we actually care about). For the standard map, for order 2 and 3 you can see visually where the UPOs should be due to the KAM islands. I take this as good as analytic knowledge.

Using Set is a better option yes. The more you re-use existing code the better.

Copy link
Member

@Datseris Datseris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should rename all "upo" to "po" like "upo" datastracture. As far as I can tell, all algorithms here work for any type of orbit?

@JonasKoziorek
Copy link
Contributor Author

All currently implemented algorithms work for both stable and unstable orbits / periodic points.

@Datseris Datseris merged commit 0ecb479 into JuliaDynamics:main Jun 11, 2024
2 checks passed
@Datseris
Copy link
Member

@JonasKoziorek you now have admin access rights to ChaosTools.jl, you can branch from origin instead of using your own fork. Personally I find this easier to work with, but you can do however you prefer with your own fork or origin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants