angular-momentum-equality task is not using ref.pos about tsid HOT 11 CLOSED

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.

from tsid.

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

from tsid.

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

from tsid.

It could be value, time_derivative,
time_second_derivative ?

from tsid.

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

from tsid.

Yes, I do agree that value seems more appropriete.

from tsid.

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

from tsid.

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

from tsid.

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?

from tsid.

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

from tsid.

Shall we close this now?

from tsid.