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

Release 1.0.0 #251

Open
jbcaillau opened this issue Jul 26, 2024 · 39 comments
Open

Release 1.0.0 #251

jbcaillau opened this issue Jul 26, 2024 · 39 comments
Assignees

Comments

@jbcaillau
Copy link
Member

jbcaillau commented Jul 26, 2024

See control-toolbox/CTDirect.jl#115 (comment)

@jbcaillau jbcaillau changed the title Release 1.0 Release 1.0.0 Aug 1, 2024
@jbcaillau
Copy link
Member Author

@ocots hadn't you written a global list of todo's for 1.0.0? can't find nothing more than scattered issues (which is fine, but would like some prioritisation)

@jbcaillau jbcaillau mentioned this issue Aug 1, 2024
@ocots
Copy link
Member

ocots commented Aug 1, 2024

@jbcaillau I can't find any list. Maybe we can do one. I watch Léon Marchand at 11:40 and then, I will start a list.

@ocots
Copy link
Member

ocots commented Aug 1, 2024

Things we can do before the new release v1.0.0.

See some info here about OptimalControl.jl.

Reference.

Documentation.

  • Document algorithms #253. We should add a description of the algorithms. In OptimalControl.jl documentation, we should add a page about Solvers and briefly describe the solve function. In CTDirect.jl we should have a more detailed presentation of the discretization scheme with the ordering of the variables...
  • Update documentation #194. 🚀 Abstract syntax documentation #155. We should add a tutorial about the abstract formulation.
  • ⚽️ NL Solver #165. We could compare MINPACK and NonlinearSolve in the tutorial about Indirect simple shooting and Goddard. We could add a test to avoid crash if using an arm system since MINPACK is not supported: see here or a simple try ... catch.

Performance.

Enhancement.

Unit tests.

Bug.

@jbcaillau
Copy link
Member Author

jbcaillau commented Aug 1, 2024

manu thanks @ocots I've added 🚀 on the must-be-done-before-1.0.0 according to me. @PierreMartinon @gergaud feel free to update!

ps. hey, #194 was the list I had in mind 👍🏽

@ocots
Copy link
Member

ocots commented Aug 1, 2024

For what I will do I have put a ⚽️

@ocots
Copy link
Member

ocots commented Aug 3, 2024

I have added MINPACK in the indirect simple shooting tutorial and the Goddard problem. I have to check that there is no error since I have added a try...catch to switch from hybrj to hybrd when an error occurs since with my computer I cannot use hybrj but hybrd is working.

@ocots
Copy link
Member

ocots commented Aug 4, 2024

We should also add a tutorial about how to improve performance installing HSL, using the best linear solver, etc.

@jbcaillau
Copy link
Member Author

I have added MINPACK in the indirect simple shooting tutorial and the Goddard problem. I have to check that there is no error since I have added a try...catch to switch from hybrj to hybrd when an error occurs since with my computer I cannot use hybrj but hybrd is working.

@ocots nice. if not already done, I guess we can remove the dependency towards NonlinearSolve 💩

@ocots
Copy link
Member

ocots commented Aug 4, 2024

No there will be both.

@jbcaillau
Copy link
Member Author

No there will be both.

where do we (still) need NonlinearSolvePoo?

@ocots
Copy link
Member

ocots commented Aug 4, 2024

No there will be both.

where do we (still) need NonlinearSolvePoo?

Tutorials:

  • Indirect simple shooting
  • Goddard

@jbcaillau
Copy link
Member Author

OK: thought you had gone back to MINPACK for these.

@ocots
Copy link
Member

ocots commented Aug 5, 2024

I think we should add:

Actually the two first points are indicated in the list above. I should add something about the Flow function.

@jbcaillau
Copy link
Member Author

jbcaillau commented Aug 5, 2024

I think we should add:

we are far from optimal right now, but that's a dev (not user) issue, including (but not limited to):

  • the user cannot do much about ordering
  • a tutorial (in manual part) about the use of the Flow function.
  • right!

Actually the two first points are indicated in the list above. I should add something about the Flow function.

@jbcaillau
Copy link
Member Author

jbcaillau commented Aug 5, 2024

To me only two things left to go 1.0.0:

Gents... 🙂

@ocots
Copy link
Member

ocots commented Aug 5, 2024

I think we should add:

Yes, we could do something like: https://docs.sciml.ai/NonlinearSolve/stable/tutorials/code_optimization/

Just simple things like you said: changing the linear solver, using MKL. Saying that it is possible to change the AD backend without any comparison. It is maybe more tuning than optimising that we should say. But I think we can profit from what @0Yassine0 did to explain some possibilities. It shows the versatility of our package.

we are far from optimal right now, but that's a dev (not user) issue, including (but not limited to):

I was not thinking of this.

  • the user cannot do much about ordering

I was simply thinking about the fact that the user can choose the method to solve its problem. For the moment, there is only one case: see

algorithms = add(algorithms, (:direct, :adnlp, :ipopt))

But, later there will be more. I thought about a simple tutorial explaining how to choose the method with its description. We could profit to redirect to the documentation of the packages we use, like ADNLPModels.jl, NLPModelsIpopt.jl and CTDirect.jl.

  • a tutorial (in manual part) about the use of the Flow function.
  • right!

Actually the two first points are indicated in the list above. I should add something about the Flow function.

@jbcaillau
Copy link
Member Author

I was simply thinking about the fact that the user can choose the method to solve its problem. For the moment, there is only one case: see

algorithms = add(algorithms, (:direct, :adnlp, :ipopt))

But, later there will be more. I thought about a simple tutorial explaining how to choose the method with its description. We could profit to redirect to the documentation of the packages we use, like ADNLPModels.jl, NLPModelsIpopt.jl and CTDirect.jl.

@ocots @PierreMartinon we should immediately add MadNLP: #259 Works out of the box (check #260) and @frapac can help in case of need 🙂

@PierreMartinon
Copy link
Member

PierreMartinon commented Aug 5, 2024

Regarding MadNLP, I have one question: I don't know how to have the CTSolveExt extension loaded if either NLPModelsIpopt or MadNLP are used. From the Julia doc, a package extension will load if all its dependencies are present.

We could try to have one extension per solver, but this is not how we currently work: we have a single function with a Symbol argument for the solver, not several methods with multiple dispatch...

@PierreMartinon
Copy link
Member

PierreMartinon commented Aug 5, 2024

For the sparsity pattern, the current trapeze method makes it a bit more involved: we typically store the dynamics at the end of a time step before moving on to the next step, so that we don't compute it twice. This complicates the decomposition of the constraints as a simple loop over the time steps, and the resulting sparsity pattern.

Here: https://github.com/control-toolbox/CTDirect.jl/blob/a9ae483a57382d44dba0612af914d78275fd6d7a/src/problem.jl

    args_i = ArgsAtTimeStep(xu, docp, 0, v)
    args_ip1 = ArgsAtTimeStep(xu, docp, 1, v)

    index = 1 # counter for the constraints
    for i in 0:docp.dim_NLP_steps-1

        # state equation
        index = setStateEquation!(docp, c, index, (args_i, args_ip1))
        # path constraints 
        index = setPathConstraints!(docp, c, index, args_i, v)

        # smart update for next iteration
        if i < docp.dim_NLP_steps-1
            args_i = args_ip1
            args_ip1 = ArgsAtTimeStep(xu, docp, i+2, v)
        end
    end

Ideally we would only have args_i and no update step. Maybe we should implement the implicit midpoint method and work from there.

@ocots
Copy link
Member

ocots commented Aug 5, 2024

@PierreMartinon Est-ce clair ?

Regarding MadNLP, I have one question: I don't know how to have the CTSolveExt extension loaded if either NLPModelsIpopt or MadNLP are used. From the Julia doc, a package extension will load if all its dependencies are present.

We could try to have one extension per solver, but this is not how we currently work: we have a single function with a Symbol argument for the solver, not several methods with multiple dispatch...

Capture d’écran 2024-08-05 à 17 57 16

La première fonction solve se trouve dans les sources tandis que celle sur les types concrets se trouvent dans 2 extensions séparées.

Bon je me suis planté pour l'appel avec IpoptSolver mais tu vois l'idée.

@jbcaillau
Copy link
Member Author

We could try to have one extension per solver [...]

@PierreMartinon that is what I would favor, again to avoid load time issues (= just load the relevant extension when triggered by the chosen solver).

@jbcaillau
Copy link
Member Author

For the sparsity pattern, the current trapeze method makes it a bit more involved: we typically store the dynamics at the end of a time step before moving on to the next step, so that we don't compute it twice. This complicates the decomposition of the constraints as a simple loop over the time steps, and the resulting sparsity pattern.

@PierreMartinon oh, right. spares half the computation. nice.

@ocots
Copy link
Member

ocots commented Aug 6, 2024

@PierreMartinon bien sûr le solve correspond à ton solve_docp.

Voui. La je coince sur ces ù*^$ de descriptions: je voudrais rajouter et tester le cas :madnlp au lieu de :ipopt, mais je ne trouve pas la bonne syntaxe -_-
J'ai essaye de completer available_methods et de passer :madnlp au solve, mais pas moyen. C'est une liste de couples, ou bien une liste tout court ?

algorithms=()
algorithms = add(algorithms, (:adnlp, :ipopt))
algorithms = add(algorithms, (:adnlp, :madnlp))
available_methods()
MethodError: Cannot `convert` an object of type 
  Tuple{Tuple{Symbol,Symbol},Tuple{Symbol, Symbol}} to an object of type 
  Tuple{Tuple{Vararg{Symbol}}}

Pas sur non plus de comprendre comment fonctionne getFullDescription avec ses priorites.

Finalement est-ce qu'on ne ferait pas mieux de revenir a de bons vieux kwargs avec des valeurs par defaut ?

Edit: bon j'ai juste enleve le type de available_methods() et ca semble marcher

@PierreMartinon
Copy link
Member

First madnlp tests control-toolbox/CTDirect.jl#191

@jbcaillau
Copy link
Member Author

jbcaillau commented Aug 6, 2024

To me only two things left to go 1.0.0:

Gents... 🙂

@PierreMartinon one step away from 1.0.0 🤞🏽

control-toolbox/CTDirect.jl#184

@ocots
Copy link
Member

ocots commented Aug 7, 2024

@PierreMartinon bien sûr le solve correspond à ton solve_docp.

Voui. La je coince sur ces ù*^$ de descriptions: je voudrais rajouter et tester le cas :madnlp au lieu de :ipopt, mais je ne trouve pas la bonne syntaxe -_- J'ai essaye de completer available_methods et de passer :madnlp au solve, mais pas moyen. C'est une liste de couples, ou bien une liste tout court ?

algorithms=()
algorithms = add(algorithms, (:adnlp, :ipopt))
algorithms = add(algorithms, (:adnlp, :madnlp))
available_methods()
MethodError: Cannot `convert` an object of type 
  Tuple{Tuple{Symbol,Symbol},Tuple{Symbol, Symbol}} to an object of type 
  Tuple{Tuple{Vararg{Symbol}}}

Pas sur non plus de comprendre comment fonctionne getFullDescription avec ses priorites.

Finalement est-ce qu'on ne ferait pas mieux de revenir a de bons vieux kwargs avec des valeurs par defaut ?

Edit: bon j'ai juste enleve le type de available_methods() et ca semble marcher

Désolé le bon type était : Tuple{Vararg{Tuple{Vararg{Symbol}}}}.

@jbcaillau
Copy link
Member Author

@ocots @PierreMartinon thanks for the active cleaning / upgrading (solve and everything) 👍🏽. was beach / boat day on may side 🌞

@PierreMartinon
Copy link
Member

The parsing for the constraints multipliers should be up in the next release. Untested though.

Before 1.0 I'd like to

  • add a second discretization scheme (implicit midpoint). Adding other schemes later should be easier.
  • then try to pass the sparsity structure and see what performance we get.

@jbcaillau
Copy link
Member Author

jbcaillau commented Aug 9, 2024

The parsing for the constraints multipliers should be up in the next release. Untested though.

Before 1.0 I'd like to

  • add a second discretization scheme (implicit midpoint). Adding other schemes later should be easier.

Not a priority, I think. I am not expecting big changes from midpoint (dual to trapezoidal scheme with more or less the same properties). Having a higher order would be nice (see control-toolbox/CTDirect.jl#101 (comment)) but not top priority for most i use case.

But even more important (see this other exchange with @amontoison, control-toolbox/CTDirect.jl#188 (comment)) Check control-toolbox/CTBase.jl#232

@PierreMartinon
Copy link
Member

I know, but I can't really pass the sparse structure with the trapeze method (more blocks than a true single step method due to each dynamics acting on two steps and not just one, and also the saving of this dynamics for the next step complicates things).

@jbcaillau
Copy link
Member Author

jbcaillau commented Aug 9, 2024

@PierreMartinon ah ok. but I think you can. independently on how you (efficiently) you compute the constraint, its value is sth like

$$x_{i+1} - x_i - (t_{i+1} - t_i) / 2 (f(x_i, u_i) + f(x_{i+1},u_{i+1}))$$

for which the derivative wrt. $x_i$'s and $u_i$'s is clear (and structured). Agree?

@jbcaillau
Copy link
Member Author

@PierreMartinon available for a quick call to decide on publishing 1.0?

@PierreMartinon
Copy link
Member

@PierreMartinon available for a quick call to decide on publishing 1.0?

Eh, more likely next week. Do we have a deadline for 1.0, conf or something ?

@jbcaillau
Copy link
Member Author

@PierreMartinon available for a quick call to decide on publishing 1.0?

Eh, more likely next week. Do we have a deadline for 1.0, conf or something ?

@PierreMartinon no deadline, just not bad to proceed according to what was planned above. next Monday? 15:00?

@PierreMartinon
Copy link
Member

@PierreMartinon available for a quick call to decide on publishing 1.0?

Eh, more likely next week. Do we have a deadline for 1.0, conf or something ?

@PierreMartinon no deadline, just not bad to proceed according to what was planned above. next Monday? 15:00?

@jbcaillau Bump

@jbcaillau
Copy link
Member Author

@PierreMartinon meeting scheduled tonight 21:00 https://univ-cotedazur.zoom.us/my/caillau

To discuss:

@ocots @gergaud please join if you're around 🤞🏾

@PierreMartinon
Copy link
Member

Breakage seems to work: control-toolbox/CTDirect.jl#209 (comment)

@PierreMartinon
Copy link
Member

Constraints / multipliers parsing seems to work: control-toolbox/CTDirect.jl#210

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

No branches or pull requests

3 participants