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

Dist diagonal #120

Closed
wants to merge 3 commits into from
Closed

Conversation

denizhanpak
Copy link
Contributor

Hey, I think I implemented this right and tested it. I ran into an issue with dynamical systems base where CoupledODEs couldn't be found so couldn't add a test for that case. Also this is my first time contributing so if I am missing steps please let me know.

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.

Thank you very much! I've left some comments that need to be addressed however!

Comment on lines +124 to +126
average_displacement(x, τ, m) -> S_m(τ)
Determines the average displacement from the diagonal generalized SSR given a τ.
Adapted from: https://www.delucafoundation.org/download/bibliography/de-luca/061.pdf.
Copy link
Member

Choose a reason for hiding this comment

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

What is SSR? Please expand it fully and avoid abbreviations.

Additionally, please properly cite the actual paper using the syntax that is used in the rest of the docstrings. [^Rosenstein1993].

@@ -13,14 +13,15 @@ diffeq = (atol = 1e-9, rtol = 1e-9)
@test exponential_decay_fit(x, y, :small) ≈ 5

henon_rule(x, p, n) = SVector(1.0 - p[1]*x[1]^2 + x[2], p[2]*x[1])
ds = DeterministicIteratedMap(henon_rule, zeros(2), [1.4, 0.3])
ds = DiscreteDynamicalSystem(henon_rule, zeros(2), [1.4, 0.3])
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change this?

data = trajectory(ds, 1000)[1]
data = trajectory(ds, 1000)#[1]
Copy link
Member

Choose a reason for hiding this comment

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

This is an odd change... You are going from DynamicalSystems.jl v3 to v2? Why? No regressions should happen in this PR. Please update your local code to DYnamicalSystems.jl v3 and adjust accordingly.


Method requires the embedding dimension as arg m.
"""
function average_displacement(x::Vector, τ::Int; m::Int=7)
Copy link
Member

Choose a reason for hiding this comment

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

Does x have to be a Vector? I think it can be AbstractVector instead.

@Datseris
Copy link
Member

Datseris commented Jul 7, 2023

Bump @denizhanpak

1 similar comment
@Datseris
Copy link
Member

Datseris commented Aug 7, 2023

Bump @denizhanpak

@denizhanpak denizhanpak deleted the dist_diagonal branch September 22, 2023 03:09
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