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
858 stars 174 forks source link

Abstraction for the State classes - [merged] #442

Closed wxmerkt closed 5 years ago

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 15:32

Merges state-api -> devel

This PR represents the first action regarding the creation of abstract classes for the core classes (reported here #74). Here I have been focused on State classes, these are the tasks:

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 15:35

changed title from Abstraction for the State classes to Abstraction for the State classes{+, #74+}

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 15:35

changed the description

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 15:41

changed title from Abstraction for the State classes{-, #74-} to Abstraction for the State classes

wxmerkt commented 5 years ago

In GitLab by @nmansard on Feb 6, 2019, 17:39

Sorry. I thought I already said: no ABC in Python please, we don't need it and I don't want that we spend 3 days in putting that everywhere. I should maybe have introduce that in the beginning. Now is too late. That is verbose code for no functionality.

wxmerkt commented 5 years ago

In GitLab by @nmansard on Feb 6, 2019, 17:42

please keep the nq,nv,nx,ndx = ... with then shorter lines. You will find this pattern everywhere in the code and I prefer it that way. Short code often equals clean code often equal more reliable code.

wxmerkt commented 5 years ago

In GitLab by @nmansard on Feb 6, 2019, 17:43

Thanks for this contribution. Nice job of documenting. Please remove ABC, we don't need it at this point. Please refrain to modify the code when not necessary.

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 18:15

changed this line in version 2 of the diff

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 18:16

changed this line in version 2 of the diff

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 18:16

added 2 commits

Compare with previous version

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 18:17

Sorry this part wasn't clear to me. I removed ABC stuff

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 18:18

I reverted these changes as requested.

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 18:18

resolved all discussions

wxmerkt commented 5 years ago

In GitLab by @cmastalli on Feb 6, 2019, 18:19

@nmansard you can merge this PR since I resolved all discussions as requested.

wxmerkt commented 5 years ago

In GitLab by @nmansard on Feb 6, 2019, 20:06

mentioned in commit f85f063310d4eea1aa27e62096d978cbde1bde6d

wxmerkt commented 5 years ago

In GitLab by @nmansard on Feb 6, 2019, 20:06

merged