search

loco-3d / crocoddyl

Crocoddyl is an optimal control library for robot control under contact sequence. Its solver is based on various efficient Differential Dynamic Programming (DDP)-like algorithms
BSD 3-Clause "New" or "Revised" License
845 stars 173 forks source link

Rk4 --- Ready to merge and waiting for review - [merged] #421

Closed wxmerkt closed 4 years ago

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 27, 2019, 21:26

Merges rk4 -> devel

I added the Linear dynamics and checked the derivatives of the dynamics and costs. The first order derivatives of L and F work.

I implemented the second order derivatives of L as well, however, there is no test yet. 1) The current test, by using the residuals doesn't make sense to me for this integration scheme. 2) I should implement an FD to unittest Lxx, Lxu etc (which should atlleast work for the case of DAMLQR). That is what I am doing now.

In case you wish to merge it now before the school, you could go ahead.

Edit: I added FD on the cost derivatives as well.

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 27, 2019, 21:28

Deals with #55

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 28, 2019, 00:20

added 10 commits

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 28, 2019, 17:10

What is the current status of this MR? If the work is asleep, should we close it yet, or mark it as in sleep mode?

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 28, 2019, 18:35

added 1 commit

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 28, 2019, 18:35

changed the description

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 28, 2019, 18:37

This MR is done and ready to merge.

I don't know how useful RK4 would be for DDP, since the second order derivatives vastly increase the number of matrix multiplications.

We can approximate as well. However, currently the derivatives are exact (for linear dynamics like DAMLQR) and working.

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 28, 2019, 19:04

changed title from Rk4 to Rk4{+ --- Ready to merge and waiting for review+}

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 28, 2019, 19:10

added 1 commit

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 29, 2019, 08:42

I find strange that you are introducing complex cost classes here. You should rather have a minimal LQR examples that is stand alone. I don't see the interest of reusing complex code here.

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 29, 2019, 08:43

please comment the function. I dont understand what it is doing. What about putting a readable name? What about adding it to the LQR class?

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 29, 2019, 08:44

I think this class should go in the main library. We already have ActionModelLQR there. It would be useful to have the DAMLQR as well.

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 29, 2019, 08:45

This is useless if remove the cost model, that I advise.

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 10:13

This is just to get a random orthonormal matrix. I only added it for the unit test. The source is https://github.com/scipy/scipy/pull/5622/files

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 10:51

If we put this in the main library, we have to agree on the convention. To match the second order dynamics eqution in DAM, I am implementing ddy = Ady + By + Cu here. (x = [y dy])

Is that good for you?

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 10:53

I can't use the cost models implemented in the main library, because they depend on pinocchioModel. So I needed an equivalent model for the vector space so that the classes in the main library can access the sizes.

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 11:08

added 1 commit

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 11:10

I added a comment for the function.

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 11:15

changed this line in version 6 of the diff

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 11:15

changed this line in version 6 of the diff

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 11:15

added 1 commit

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 11:16

Done. I removed Vectormodel

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 11:18

I need to rewrite the CostModelState here because the class implemented inside the main code is dependent on pinocchio.

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 29, 2019, 13:11

name it randomOrthogonalMatrix

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 29, 2019, 13:11

Yes I am

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 29, 2019, 13:15

I am sorry I do not understand your answer. My point was that DAMLQR should be a very simple class not relying on CostModel to compute the result. The cost should just be l(q,v,u) = qQq + vVv + uUu with Q,V,U three positive matrices. Simple to implement in calc, no need to rely on CostModelState or any other CostModelXXX. Is it not?

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 13:19

Ah ok. I'll do that.

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 14:25

changed this line in version 7 of the diff

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 14:25

changed this line in version 7 of the diff

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 14:25

added 21 commits

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 29, 2019, 14:25

Done.

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 09:48

Is this MR ok? Should I merge it in devel?

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 16:03

added 79 commits

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 16:03

marked as a Work In Progress from rbudhira/cddp@cdfd46e06fcfa15504be22808f0959bbc2978fb5

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 16:03

changed target branch from topic/refact to devel

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 16:04

unmarked as a Work In Progress

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 16:06

added 1 commit

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 30, 2019, 17:31

ncost should be the len of the costresiduals. If you dont have residuals, then don't set up a ncost.

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 30, 2019, 17:32

data.Fx[:nv,:] = A data.Fx[nv:,:] = B

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 30, 2019, 17:33

use v or vq instead of dq

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 30, 2019, 17:33

i think you should add a drift term to be generic: a = Ax + Bv + Cu + d

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 30, 2019, 17:35

Could you add a disclaimer that this model is not yet heavily tested, and we are not sure that it is the way to go wrt performances?

wxmerkt commented 4 years ago

In GitLab by @nmansard on Jan 30, 2019, 17:36

Doc should go in the first line of the function, as a string.

def randomOM(): '''blablabla'''

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 19:03

changed this line in version 11 of the diff

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 19:03

changed this line in version 11 of the diff

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 19:03

changed this line in version 11 of the diff

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 19:03

added 2 commits

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 19:04

Done.

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 19:06

changed this line in version 12 of the diff

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 19:06

added 1 commit

Compare with previous version

wxmerkt commented 4 years ago

In GitLab by @proyan on Jan 30, 2019, 19:06

Done.