pyomeca / bioptim

An optimization framework that links CasADi, Ipopt, ACADOS and biorbd for Optimal Control Problem
MIT License
95 stars 47 forks source link

friction coefficient sign #872

Closed Ipuch closed 5 months ago

Ipuch commented 7 months ago

There is no constraint on the sign of the friction coefficient.

I would argue to make it positive only here: https://github.com/pyomeca/bioptim/blob/e72d242bfc082f3e11b4a4c9b56dfb809d75704d/bioptim/models/biorbd/biorbd_model.py#L20C1-L36C84

because there may be a flaw here: https://github.com/pyomeca/bioptim/blob/e72d242bfc082f3e11b4a4c9b56dfb809d75704d/bioptim/dynamics/dynamics_functions.py#L148-L153

To me, it should be a minus, because the torque is applied against the direction of motion

tau = tau - nlp.model.friction_coefficients @ qdot if with_friction else tau

Plus, I've checked the example using it, there is no negative sign on the reaching arm example, meaning the joint velocities help to actuate the motion.

https://github.com/pyomeca/bioptim/blob/e72d242bfc082f3e11b4a4c9b56dfb809d75704d/bioptim/examples/stochastic_optimal_control/arm_reaching_torque_driven_collocations.py#L107-L118

I don't want to take action before having your takes on this @EveCharbie.

pariterre commented 7 months ago

Please note that the friction feature will be added very soon in biorbd which may make this "friction_coefficients" useless when using biorbd

EveCharbie commented 7 months ago

Seems like you found a good bug @Ipuch :) I agree with what you suggest to do, thanks!

pariterre commented 7 months ago

Friction coefficient is now vanilla in biorbd thanks to https://github.com/pyomeca/biorbd/pull/350

EveCharbie commented 7 months ago

Really nice @pariterre, thanks! Will you also add segments_to_apply_external_forces ? ;)