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

OCP Croco Class #112

Open
wants to merge 116 commits into
base: topic/humble-devel/refactor
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
116 commits
Select commit Hold shift + click to select a range
2b74b10
creating the ocp param base class & the ocp base croco
ArthurH91 Dec 11, 2024
60648fc
populate ocp param & croco
ArthurH91 Dec 11, 2024
3d36d16
setting up the params base class
ArthurH91 Dec 11, 2024
0820ea7
add test for TestOcpBase
ArthurH91 Dec 11, 2024
16fbafe
change all relative paths to absolute ones
ArthurH91 Dec 11, 2024
6ff4507
unit tests of the data class ocp param croco base
ArthurH91 Dec 11, 2024
1da7779
first try on testing the ocp croco base class
ArthurH91 Dec 11, 2024
0105d0c
ocp croco set up
ArthurH91 Dec 11, 2024
4ddeea4
getting rid of target in the dataclass
ArthurH91 Dec 11, 2024
b2eac48
getting rid of target in the dataclass 2
ArthurH91 Dec 11, 2024
4f07f49
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 11, 2024
61beafa
fixing imports
ArthurH91 Dec 12, 2024
5d0ced3
fixing imports 2
ArthurH91 Dec 12, 2024
6be9c9f
removing x0 property ocp base
ArthurH91 Dec 12, 2024
bf1dde7
Update agimus_controller/agimus_controller/trajectory.py
ArthurH91 Dec 12, 2024
4c42f41
Update agimus_controller/agimus_controller/ocp_base.py
ArthurH91 Dec 12, 2024
eab8f12
docstring + bringing back x0 in the solve + changing the name x_init …
ArthurH91 Dec 13, 2024
894fce4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 13, 2024
09c47cc
adding armature in the robot model factory
ArthurH91 Dec 13, 2024
cf7991f
Merge branch 'ahaffemayer/feature/create-ocp-param-class' of github.c…
ArthurH91 Dec 13, 2024
a55807d
getting rid of the x0 property
ArthurH91 Dec 13, 2024
f7db9b6
getting rid of x init and u init method
ArthurH91 Dec 13, 2024
92212d2
changing the robot model input for the init
ArthurH91 Dec 13, 2024
0e8d145
getting rid of the trajectory points
ArthurH91 Dec 13, 2024
95ab7c7
getting rid of useless imports
ArthurH91 Dec 13, 2024
ef8bd36
changing the solve function to only solving and creating properties f…
ArthurH91 Dec 13, 2024
5cda790
changing the result of solve
ArthurH91 Dec 13, 2024
6e54b6e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 13, 2024
58b8dc7
changing OCPParam to ocp_param
ArthurH91 Jan 2, 2025
3faa492
adding few comments + changing the docstring for the init
ArthurH91 Jan 2, 2025
4e2373c
adding the filter linesearch data input in the dataclass ocp param croco
ArthurH91 Jan 2, 2025
9047baa
unit testing the ocp param base dataclass
ArthurH91 Jan 2, 2025
ebb28b7
deleting the ocp base unit test because that's only an abstract class
ArthurH91 Jan 2, 2025
90afd0f
changing ocpparamcrocobase to ocpparambasecroco
ArthurH91 Jan 2, 2025
1fb5f38
wrong placement of problem solved
ArthurH91 Jan 2, 2025
c544c12
changing ocpparamcrocobase to ocpparambasecroco
ArthurH91 Jan 2, 2025
e422536
switching from ... to pass for abstract class because it explodes oth…
ArthurH91 Jan 2, 2025
fa3da6e
fixing the union problem
ArthurH91 Jan 2, 2025
5c8a53e
fixing the property
ArthurH91 Jan 2, 2025
038cd0f
first unit test passing, now working on the simple example as unit test
ArthurH91 Jan 2, 2025
8ce22ad
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 2, 2025
156afd8
addind setters for ocp base croco & robot model
ArthurH91 Jan 6, 2025
f6c8efb
adding the unit test for the simple ocp
ArthurH91 Jan 6, 2025
982f976
adding the npy results for the simple ocp used for the unit testing
ArthurH91 Jan 6, 2025
31c06ff
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 6, 2025
e011080
Update agimus_controller/agimus_controller/ocp_base_croco.py
ArthurH91 Jan 6, 2025
bc1f31d
Update agimus_controller/agimus_controller/ocp_base_croco.py
ArthurH91 Jan 6, 2025
255c1f0
Update agimus_controller/agimus_controller/ocp_base.py
ArthurH91 Jan 6, 2025
417d9bb
Update agimus_controller/agimus_controller/ocp_base.py
ArthurH91 Jan 6, 2025
bafd67c
Update agimus_controller/agimus_controller/ocp_base_croco.py
ArthurH91 Jan 6, 2025
54ddd9b
Update agimus_controller/agimus_controller/ocp_base_croco.py
ArthurH91 Jan 6, 2025
adb3ced
Update agimus_controller/agimus_controller/ocp_base_croco.py
ArthurH91 Jan 6, 2025
861f7c2
Update agimus_controller/agimus_controller/ocp_base_croco.py
ArthurH91 Jan 6, 2025
db440c7
Apply suggestions from code review
ArthurH91 Jan 6, 2025
b2f99bb
Update agimus_controller/agimus_controller/ocp_param_base.py
ArthurH91 Jan 6, 2025
ee4181c
taking reviews in to account
ArthurH91 Jan 6, 2025
841493e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 6, 2025
402968b
switching to snake case
ArthurH91 Jan 6, 2025
d6ca98b
fixing typo
ArthurH91 Jan 6, 2025
55c3095
change t to horizon size
ArthurH91 Jan 6, 2025
dcf4812
Merge branch 'ahaffemayer/feature/create-ocp-param-class' of github.c…
ArthurH91 Jan 6, 2025
b8eb8ab
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 6, 2025
72e2185
methods instead of properties for the croco models
ArthurH91 Jan 6, 2025
b6266d0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 6, 2025
303dd30
Update agimus_controller/agimus_controller/ocp_base_croco.py
ArthurH91 Jan 7, 2025
0dfe450
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 7, 2025
6b731ca
Update agimus_controller/tests/test_ocp_param_base.py
ArthurH91 Jan 7, 2025
7c703d6
Update agimus_controller/tests/test_ocp_param_base.py
ArthurH91 Jan 7, 2025
25d8344
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 7, 2025
d2b9107
Update agimus_controller/tests/test_ocp_croco_base.py
ArthurH91 Jan 7, 2025
ad39d40
Update agimus_controller/agimus_controller/ocp_base.py
ArthurH91 Jan 7, 2025
2885b0a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 7, 2025
8083c11
getting rid of useless docstrings
ArthurH91 Jan 7, 2025
c1f6126
displacing the creation of running model & terminal model in the if s…
ArthurH91 Jan 7, 2025
0cc5936
proper type hinting
ArthurH91 Jan 7, 2025
9e5c374
fixing unit tests
ArthurH91 Jan 7, 2025
d9e49cc
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 7, 2025
88d2449
Update agimus_controller/agimus_controller/ocp_base.py
ArthurH91 Jan 7, 2025
2aa81eb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 7, 2025
88f572d
Update agimus_controller/agimus_controller/ocp_base_croco.py
ArthurH91 Jan 7, 2025
c7cb95c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 7, 2025
91bf33c
adressing the change in update crocoddyl problem
ArthurH91 Jan 7, 2025
b52e01d
merge conflict
ArthurH91 Jan 7, 2025
e29276f
changing the input of the update croccodyl problem method
ArthurH91 Jan 7, 2025
e3ba9e6
first draft on the modification of robot model factory
ArthurH91 Jan 8, 2025
a097d30
adding some tests + reformulating the code a bit
ArthurH91 Jan 8, 2025
3ba81d4
recursivity problems fixed
ArthurH91 Jan 8, 2025
57b2f36
Update agimus_controller/agimus_controller/factory/robot_model.py
ArthurH91 Jan 9, 2025
ce26d70
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 9, 2025
b2afd0f
changing the order of the methods
ArthurH91 Jan 9, 2025
1c8a1e3
Update agimus_controller/agimus_controller/factory/robot_model.py
ArthurH91 Jan 9, 2025
70a73ee
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 9, 2025
393ff30
add the color as parameter
ArthurH91 Jan 9, 2025
406a689
correcting typo in type for urdf
ArthurH91 Jan 9, 2025
12d17ef
unit testing the dataclass
ArthurH91 Jan 9, 2025
80062e6
unit testing the dataclass2
ArthurH91 Jan 9, 2025
720cf23
wrapping up unit testing for rmf
ArthurH91 Jan 10, 2025
f5fed68
getting rid of explicit casting
ArthurH91 Jan 10, 2025
ea73d2e
Update agimus_controller/agimus_controller/factory/robot_model.py
ArthurH91 Jan 10, 2025
620f4b0
typo
ArthurH91 Jan 10, 2025
2c55b75
free flyer exception in the unit test of armature
ArthurH91 Jan 10, 2025
a91e554
bring back path objects
ArthurH91 Jan 10, 2025
294f5b9
integration test to check that model is working well with pinocchio
ArthurH91 Jan 10, 2025
4a7cedd
merging robot refactor
ArthurH91 Jan 10, 2025
d805ea4
fixing unit test ocp croco base param
ArthurH91 Jan 10, 2025
d53b0f6
Update agimus_controller/tests/test_robot_models.py
ArthurH91 Jan 10, 2025
086bcca
Update agimus_controller/agimus_controller/factory/robot_model.py
ArthurH91 Jan 10, 2025
3e59282
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 10, 2025
ca1066b
getting rid of str in paths + collision color in param
ArthurH91 Jan 10, 2025
488ab70
fixing the tests
ArthurH91 Jan 10, 2025
b3c121b
changing locked joint into var
ArthurH91 Jan 10, 2025
a3ee5c5
self collision test improved
ArthurH91 Jan 10, 2025
6fcf107
test reduced model
ArthurH91 Jan 10, 2025
3ddf226
Merge branch 'ahaffemayer/refactor-robot-factory' into ahaffemayer/fe…
ArthurH91 Jan 10, 2025
d6caca3
changing factory to models
ArthurH91 Jan 10, 2025
faf187e
first two unit tests working
ArthurH91 Jan 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion agimus_controller/agimus_controller/mpc_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import numpy as np
import numpy.typing as npt

from agimus_controller.trajectory import TrajectoryPoint
from agimus_controller.agimus_controller.trajectory import TrajectoryPoint
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved


@dataclass
Expand Down
28 changes: 19 additions & 9 deletions agimus_controller/agimus_controller/ocp_base.py
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from abc import ABC, abstractmethod
from typing import List
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved

import numpy as np
import numpy.typing as npt

from agimus_controller.mpc_data import OCPResults, OCPDebugData
from agimus_controller.trajectory import WeightedTrajectoryPoint
from agimus_controller.agimus_controller.mpc_data import OCPResults, OCPDebugData
from agimus_controller.agimus_controller.trajectory import WeightedTrajectoryPoint
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved


class OCPBase(ABC):
Expand All @@ -13,30 +16,37 @@ def __init__(self) -> None:
def set_reference_horizon(
self, reference_trajectory: list[WeightedTrajectoryPoint]
) -> None:
...
pass
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved

@abstractmethod
@property
def horizon_size() -> int:
...
pass

@abstractmethod
@property
def dt() -> int:
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
...
pass

@abstractmethod
@property
def x0() -> npt.NDArray[np.float64]:
pass
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved

@abstractmethod
def solve(
self, x0: np.ndarray, x_init: list[np.ndarray], u_init: list[np.ndarray]
self,
x_init: List[npt.NDArray[np.float64]],
u_init: List[npt.NDArray[np.float64]],
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
) -> None:
...
pass

@abstractmethod
@property
def ocp_results(self) -> OCPResults:
...
pass

@abstractmethod
@property
def debug_data(self) -> OCPDebugData:
...
pass
226 changes: 226 additions & 0 deletions agimus_controller/agimus_controller/ocp_base_croco.py
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
from abc import ABC, abstractmethod
from typing import List

import crocoddyl
import mim_solvers
import numpy as np
import pinocchio as pin

from agimus_controller.agimus_controller.mpc_data import OCPResults, OCPDebugData
from agimus_controller.agimus_controller.trajectory import WeightedTrajectoryPoint
from agimus_controller.agimus_controller.ocp_base import OCPBase
from agimus_controller.agimus_controller.ocp_param_base import OCPParamsCrocoBase
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved


class OCPCrocoBase(OCPBase):
def __init__(
self,
rmodel: pin.Model,
cmodel: pin.GeometryModel,
OCPParams: OCPParamsCrocoBase,
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO OCPParamsCrocoBase should be simply arguments of the __init__ function. In C++ it's required due to registers and stuff, but python doesn't care

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i believe that having a data class is better for readability / organisation purpose, and so that you don't have gazillion of stuff in the init of the class

Copy link
Contributor

Choose a reason for hiding this comment

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

But most of them can be passed as defaults. And this is a common pattern in python libraries. Just look at how many parameters matplotlib function have

ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
) -> None:
"""Creates an instance of the OCPCrocoBase class. This is an example of an OCP class that inherits from OCPBase,
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
for a simple goal reaching task. The class is used to solve the Optimal Control Problem (OCP) using the Crocoddyl library.
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved

Args:
rmodel (pin.Model): Robot model.
cmodel (pin.GeometryModel): Collision Model of the robot.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
rmodel (pin.Model): Robot model.
cmodel (pin.GeometryModel): Collision Model of the robot.
robot_model (RobotModelFactory): All models of the robot.

OCPParams (OCPParamsBase): Input data structure of the OCP.
"""
self._rmodel = rmodel
self._cmodel = cmodel
self._OCPParams = OCPParams

@abstractmethod
MaximilienNaveau marked this conversation as resolved.
Show resolved Hide resolved
@property
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
def horizon_size(self) -> int:
"""Number of time steps in the horizon."""
return self._OCPParams.T
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be obtained from SolverBase.problem.T + 1

Copy link
Collaborator

Choose a reason for hiding this comment

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

but we don't have access to it before we set the solver so I guess it's fine this way ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. But if you set up the problem in the __init__ you will always have it set up


@abstractmethod
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@abstractmethod

There is no need for @abstractmethd

@property
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
def dt(self) -> np.float64:
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
"""Integration step of the OCP."""
return self._OCPParams.dt
Copy link
Contributor

Choose a reason for hiding this comment

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

In theory this can be obtained from SolverBase.problem.RunningModels[0].dt, but this makes an assumption that the fist running model is of a type IntegratedActionModelAbstract and all running models have the same dt. Other option I would suggest is to store dt as a class parameter and return self._dt

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

but we can obtain it from the params class, and for clarity's sake, i believe that having many methods returning the same value may be problematic at some point


@abstractmethod
@property
def x0(self) -> np.ndarray:
"""Initial state of the robot."""
return self._OCPParams.WeightedTrajectoryPoints[0].point.robot_configuration
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is wrong.
The WeightedTrajectoryPoints is meant as references not initial state.
x0 is coming from the robot sensors while WeightedTrajectoryPoints comes from the planner (or higher entity).
I would remove this method all together.

Copy link
Contributor

Choose a reason for hiding this comment

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

I second removing this method


@abstractmethod
def x_init(self) -> np.ndarray:
Copy link
Collaborator

Choose a reason for hiding this comment

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

again the ocp don't need this is comes in from the warmstart.

"""Initial guess of the states."""
x_init = []
for TrajectoryPoint in self._OCPParams.WeightedTrajectoryPoints:

Choose a reason for hiding this comment

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

snake_case: "TrajectoryPoint" -> "trajectory_point" or "traj_point" or "t_point" etc.

Copy link
Collaborator

Choose a reason for hiding this comment

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

little preference for "trajectory_point"

x = np.array(
TrajectoryPoint.point.robot_configuration
+ TrajectoryPoint.point.robot_velocity
)
x_init.append(x)
return x_init

@abstractmethod
def u_init(self) -> np.ndarray:
Copy link
Collaborator

Choose a reason for hiding this comment

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

comes form the warmsart.

"""Initial guess of the controls."""
u_init = [
np.zeros(self._rmodel.nq) for _ in range(self.horizon_size) - 1
] # For the panda nu = nq, but i'm not sure nu exists in pinocchio
return u_init

ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
@abstractmethod
def solve(
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
self,
) -> bool:
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
"""Solves the OCP. Returns True if the OCP was solved successfully, False otherwise.

Returns:
bool: True if the OCP was solved successfully, False otherwise.
"""
### Creation of the state and actuation models

# Stat and actuation model
self._state = crocoddyl.StateMultibody(self._rmodel)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can't we initial that earlier? And not in the online part?

Copy link
Contributor

Choose a reason for hiding this comment

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

This should generally be a part of __init__

Choose a reason for hiding this comment

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

In general, I wonder if this is reasonable (in terms of time) to create croco objects (running/terminal costs, Action models etc.) from scratch every time we solve? It's simpler than going through through the croco ocp structure to only but might be a bit slow.
I just checked on a personal codebase a similar function and got:
T = 20 -> 0.7 ms
T = 40 -> 1.5 ms
T = 60 -> 2.7 ms
T = 80 -> 3.8 ms
T = 100 -> 4.5 ms

So it seems that the _state/_actuation creation is negligeable (I'd still put it in __init__ or a setup function) compared to the running cost creation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for the test @MedericFourmy! But yeah, i believe we can put as much as possible in the init

self._actuation = crocoddyl.ActuationModelFull(self._state)

self._runningModelList = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

here you should assume an abstract API that fills the self._runningModelList and the name of the variable is wrong. This is google standard not pep8.
Hence:

Suggested change
self._runningModelList = []
self._running_model_list = self._create_running_model_list()

and later on:

@abstractmethod
def _running_model_list(self) -> list[crocco.RunningModel]:
   """Create the running model list."""
   ...

# Running & terminal cost models
for t in range(self.horizon_size):
### Creation of cost terms
# State Regularization cost
xResidual = crocoddyl.ResidualModelState(self._state, self.x0)
xRegCost = crocoddyl.CostModelResidual(self._state, xResidual)

# Control Regularization cost
uResidual = crocoddyl.ResidualModelControl(self._state)
uRegCost = crocoddyl.CostModelResidual(self._state, uResidual)

# End effector frame cost
frameTranslationResidual = crocoddyl.ResidualModelFrameTranslation(
self._state,
self._OCPParams.ee_name,
self._OCPParams.WeightedTrajectoryPoints[t]
.point.end_effector_poses[self._OCPParams.ee_name]
.translation,
)

goalTrackingCost = crocoddyl.CostModelResidual(
self._state, frameTranslationResidual
)

# Adding costs to the models
if t < self.horizon_size - 1:
runningCostModel = crocoddyl.CostModelSum(self._state)
runningCostModel.addCost(
"stateReg",
xRegCost,
np.concatenate(
[
self._OCPParams.WeightedTrajectoryPoints[
t
].weight.w_robot_configuration,
self._OCPParams.WeightedTrajectoryPoints[
t
].weight.w_robot_velocity,
]
),
)
runningCostModel.addCost(
"ctrlRegGrav",
uRegCost,
self._OCPParams.WeightedTrajectoryPoints[t].weight.w_robot_effort,
)
runningCostModel.addCost(
"gripperPoseRM",
goalTrackingCost,
self._OCPParams.WeightedTrajectoryPoints[
t
].weight.w_end_effector_poses,
)
# Create Differential Action Model (DAM), i.e. continuous dynamics and cost functions
self._running_DAM = crocoddyl.DifferentialActionModelFreeFwdDynamics(
self._state,
self._actuation,
runningCostModel,
)
# Create Integrated Action Model (IAM), i.e. Euler integration of continuous dynamics and cost
runningModel = crocoddyl.IntegratedActionModelEuler(
self._running_DAM, self.dt
)
runningModel.differential.armature = OCPParamsCrocoBase.armature
self._runningModelList.append(runningModel)
else:
terminalCostModel = crocoddyl.CostModelSum(self._state)
terminalCostModel.addCost(
"stateReg",
xRegCost,
np.concatenate(

Choose a reason for hiding this comment

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

I don't think that the CostModelSum::addCost API take a vector of weights as its third input:
https://github.com/loco-3d/crocoddyl/blob/da92f67394c07c987458a8cb24bc0e33edd64227/include/crocoddyl/core/costs/cost-sum.hxx#L29

For weights regularization, you can define a CostModelResidual with an ActivationModelWeightedQuad, see here or here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thank you! I was doing it blindly here tbh

[
self._OCPParams.WeightedTrajectoryPoints[
t
].weight.w_robot_configuration,
self._OCPParams.WeightedTrajectoryPoints[
t
].weight.w_robot_velocity,
]
),
)
terminalCostModel.addCost(
"gripperPose",
goalTrackingCost,
self._OCPParams.WeightedTrajectoryPoints[
t
].weight.w_end_effector_poses,
)

self._terminal_DAM = crocoddyl.DifferentialActionModelFreeFwdDynamics(
self._state, self._actuation, terminalCostModel
)

self._terminalModel = crocoddyl.IntegratedActionModelEuler(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same story as for the runnning models. All Python variables are snake_case.
Plus you should define a function that fills this list:

@abstractmethod
def _terminal_model_list(self) -> list[crocco.TerminalModel]:
   """Create the running model list."""
   ...

and then use it in the solve function again:

    self._terminal_model_list = self._create_terminal_model_list()

self._terminal_DAM, 0.0
)
self._terminalModel.differential.armature = OCPParamsCrocoBase.armature

problem = crocoddyl.ShootingProblem(
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
self.x0, self._runningModelList, self._terminalModel
)
# Create solver + callbacks
ocp = mim_solvers.SolverSQP(problem)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we assume that you and Theo are going to use the mim_solvers all the time?
I guess yes due to constraints...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think we can have a flag in the ocp param maybe to indicate which solver use? but it might be too verbose, we could only use mim solver & csqp, which is a sqp solver when there's no constraints

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would keep it simple and set the mim_solvers as the default for everyone.
If the performance are similar I would take the most generic solver and impose this one...


# Merit function
ocp.use_filter_line_search = False
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be a parameter?


# Parameters of the solver
ocp.termination_tolerance = OCPParamsCrocoBase.termination_tolerance
ocp.max_qp_iters = OCPParamsCrocoBase.qp_iters
ocp.eps_abs = OCPParamsCrocoBase.eps_abs
ocp.eps_rel = OCPParamsCrocoBase.eps_rel

ocp.with_callbacks = OCPParamsCrocoBase.callbacks

result = ocp.solve(self.x_init, self.u_init, OCPParamsCrocoBase.solver_iters)

self.ocp_results = OCPResults(
states=ocp.xs,
ricatti_gains=ocp.Ks, # Not sure about this one
feed_forward_terms=ocp.us,
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
result = ocp.solve(self.x_init, self.u_init, OCPParamsCrocoBase.solver_iters)
self.ocp_results = OCPResults(
states=ocp.xs,
ricatti_gains=ocp.Ks, # Not sure about this one
feed_forward_terms=ocp.us,
)
result = self._ocp.solve(self.x_init, self.u_init, OCPParamsCrocoBase.solver_iters)
self._ocp_results = OCPResults(
states=ocp.xs,
ricatti_gains=ocp.Ks, # Not sure about this one
feed_forward_terms=ocp.us,
)
# fill in OCPDebugData object

This is all that this class should implement. Nothing more. Solve, and pass it to our objects. The rest should be a separate function that creates OCP and shooting problem


return result
Copy link
Contributor

Choose a reason for hiding this comment

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

This function should not return anything just like the OCPBase


@abstractmethod
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@abstractmethod

No need for that

@property
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
def ocp_results(self) -> OCPResults:
"""Output data structure of the OCP.

Returns:
OCPResults: Output data structure of the OCP. It contains the states, Ricatti gains and feed-forward terms.
"""
return self._ocp_results
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved

@abstractmethod
@property
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
def debug_data(self) -> OCPDebugData:
MaximilienNaveau marked this conversation as resolved.
Show resolved Hide resolved
...
31 changes: 31 additions & 0 deletions agimus_controller/agimus_controller/ocp_param_base.py
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
from dataclasses import dataclass
from typing import List

import numpy as np
import numpy.typing as npt

from agimus_controller.agimus_controller.trajectory import WeightedTrajectoryPoint
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved


@dataclass
class OCPParamsCrocoBase:
"""Input data structure of the OCP."""

# Data relevant to solve the OCP
dt: np.float64 # Integration step of the OCP
T: int # Number of time steps in the horizon
Copy link
Collaborator

Choose a reason for hiding this comment

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

Follow the remark on the "T" name above

solver_iters: int # Number of solver iterations
qp_iters: int = 200 # Number of QP iterations (must be a multiple of 25).
termination_tolerance: np.float64 = (
1e-3 # Termination tolerance (norm of the KKT conditions)
)
eps_abs: np.float64 = 1e-6 # Absolute tolerance of the solver
eps_rel: np.float64 = 0 # Relative tolerance of the solver
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
callbacks: bool = False # Flag to enable/disable callbacks

# Weights, costs & helpers for the costs & constraints
WeightedTrajectoryPoints: List[
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
WeightedTrajectoryPoint
] | None = None # List of weighted trajectory points
armature: npt.NDArray[np.float64] | None = None # Armature of the robot
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should go with a RobotModel object that goes into the robot_model class.
We should use the srdf of the robot for this maybe?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

srdf would be a hassle, but we could discuss it in a separate PR. For now, i simply put the armature as a property.

ee_name: str | None = None # Name of the end-effector
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion agimus_controller/agimus_controller/trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class TrajectoryPoint:
robot_configuration: np.ndarray
robot_velocity: np.ndarray
robot_acceleration: np.ndarray
robot_effort: np.ndarray
robot_effort: np.ndarray # Is robot effort the same as robot torque?
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved
forces: dict[Force] # Dictionary of pinocchio.Force
end_effector_poses: dict[SE3] # Dictionary of pinocchio.SE3

Expand Down
13 changes: 13 additions & 0 deletions agimus_controller/tests/test_ocp_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import unittest

from agimus_controller.agimus_controller.ocp_base import OCPBase
ArthurH91 marked this conversation as resolved.
Show resolved Hide resolved


class TestOCPBase(unittest.TestCase):
def test_abstract_class_instantiation(self):
with self.assertRaises(TypeError):
OCPBase()


if __name__ == "__main__":
unittest.main()
Loading