angular-momentum-equality task is not using ref.pos

[pFernbach]

The task-angular-momentum-equality take the reference momentum from the trajectorySample.vel() value (https://github.com/stack-of-tasks/tsid/blob/master/src/tasks/task-angular-momentum-equality.cpp#L98) and it's derivative from the trajectorySample.acc() value (https://github.com/stack-of-tasks/tsid/blob/master/src/tasks/task-angular-momentum-equality.cpp#L103).

I think that I understand the reasoning here that a momentum is relative to a 'velocity'. But as the name of the task suggest I would have expected to set the momentum reference from the trajectorySample.pos() value and it's derivative from the trajectorySample.vel() value.

Can I propose a PR to change this or should we keep the current behaviour ?

[andreadelprete]

I don't have a strong opinion on this, so I'd tend to agree with you, unless somebody thinks this is a good idea. I suggest to wait a couple of days to make sure nobody is against this for some good reasons, and then you can change it.

[jcarpent]

I think the naming pos, vel and acc are misleading in fact. Another naming would be expected here I would say.

[andreadelprete]

Maybe we could replace vel with rate_of_change? But what could we use to replace pos? And acc?

[pFernbach]

It could be value, time_derivative, time_second_derivative ?

[andreadelprete]

I like value! Maybe I'd go for first_time_derivative and second_time_derivative though.

[jcarpent]

Yes, I do agree that value seems more appropriete.

[andreadelprete]

@pFernbach Do you plan to do the PR? Or should I do it instead?

[pFernbach]

Sorry I forgot about, I'll do it next week. Do you confirm the following changes:

• rename TrajectorySample::pos, vel, acc to value, time_derivative, second_time_derivative
• Adapt all the C++ code
• Set the current python API with pos, vel, acc deprecated and add a new one with value, time_derivative, second_time_derivative

[andreadelprete]

Yes, that was the plan, but now that I think about it, since a trajectory is a function of time only, maybe we don't need to specify that the derivatives are wrt time. Therefore, we could simplify the names to value, derivative, second_derivative. What do you think?

[pFernbach]

Yes it make sense, I don't think that there could be any confusion.

[andreadelprete]

Shall we close this now?