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

Optimized shooting method #3

Merged
merged 59 commits into from
Sep 15, 2024
Merged

Optimized shooting method #3

merged 59 commits into from
Sep 15, 2024

Conversation

JonasKoziorek
Copy link
Contributor

I implemented recent paper that searches for UPOs via optimization (link). The paper also includes python implementation in the appendix.

My implementation has a few potential problems, that should be addressed:

  • I want to avoid creating ds in every iteration
    f = dynamic_rule(ds)
    nds = CoupledODEs(new_rule(f, T), current_state(ds), current_parameters(ds); diffeq = ds.diffeq)
    This should be possible by simply not integrating from 0 to 1 but from 0 to T. However I couldn't make it to work for some reason.
  • I would like to use other solvers than RKO65
    alg = OptimizedShooting(Δt=1/(2^6), p=3)
    ds = CoupledODEs(dynamic_rule, state, params; diffeq = (alg=RKO65(), dt=alg.Δt))
    but all other solvers I tried simply weren't working. In the paper they use simple explicit 5-th order runge kutta. I think that choice of solver might be the cause of problems in Saiki too.
  • I know that DynamicalSystemsBase.jl has OrdinaryDiffEq.jl as a dependency but the docs here say that to access the solvers, it has to be imported by the user. I wanted to change the default solver so I added the OrdinaryDiffEq.jl as a dependency again. I don't know if this is optimal. Is there a way to access the various solvers internally through DynamicalSystemsBase.jl?

@Datseris
Copy link
Member

cc also @rveltz : if you know this algorithm, let's make sure that we aren't duplicating effort. If something like this is in BFKit, or parts of the code have been tried in BFKit, let's reuse the knowledge.

@JonasKoziorek
Copy link
Contributor Author

Everything works as we discussed today. true in step was causing those problems.

@Datseris
Copy link
Member

perfect!

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.

You can optimize the code. If you don't do it until our next meeting, then I will show you how to use profiling, and we can see the big difference in profiling before and after the optimizations.

OptimizedShooting(; kwargs...)

A shooting method [Dednam2014](@cite) combined with Levenberg-Marquardt optimization
to find periodic orbits of continuous systems.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
to find periodic orbits of continuous systems.
to find periodic orbits of continuous time systems.

Comment on lines 81 to 83
R1 = compute_residual(ds, u0, T*alg.Δt, alg.n, 0)
R2 = compute_residual(ds, u0, T*alg.Δt, alg.n, T)
err = R2 .- R1
Copy link
Member

Choose a reason for hiding this comment

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

Hm, isn't this here too ineficient? Every time you call the cost function you are creating R1, R2, err. Does the optimize function run in parallel? If not, we can re-use existing vectors R1, R2, err instead of creating them all the time. For example, you can make costfunc be a type and use function-like objects. Something like

struct CostFunc
R1::Vector
R2::Vector
err:Vector
end

function (cf::CostFunc)(v, ds, alg)
   (; R1, R2, err) = cf
   # same code as in your function
end

return err
end

function compute_residual(ds, u0, Δt, n, t0)
Copy link
Member

Choose a reason for hiding this comment

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

Same story. You need to make this compute_residual!(R, ...) that uses an existing array R instead of creating a new one every time.

src/api.jl Outdated Show resolved Hide resolved
@JonasKoziorek
Copy link
Contributor Author

I tried to make it faster but with no success. Even though I reduced allocations almost in half, the runtime wasn't faster. Let's try profiling together next time then!

@JonasKoziorek
Copy link
Contributor Author

The minimal_period was messing up one of the tests so I disabled it for now. I will allow passing kwargs to minimal_period in a different PR to be able to affect the precision.

@JonasKoziorek
Copy link
Contributor Author

I suddenly can't build the docs. I attach the stacktrace.txt here. It originates in build_docs_with_style.jl line 30. Commenting this line solves the problem. Could you help me with this @Datseris?

@Datseris
Copy link
Member

docbuild problem comes from Documenter.jl. We can't resolve it :( :( :( cc @Tortar , I searched throught the JuliaDynamics/doctheme repo, we don't have $todo anywhere. It has to be from Documenter.jl.

@JonasKoziorek JonasKoziorek merged commit 64be795 into main Sep 15, 2024
1 of 2 checks passed
@JonasKoziorek JonasKoziorek deleted the optimized_shooting branch September 15, 2024 06:51
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.

2 participants