Definition of neutral configuration

[proyan]

We currently have two definition for a neutral configuration for a model. 1) model::neutralConfiguration, which can be set by srdf, and is being used for loading the half-sitting posture of a robot.

2) pinocchio::neutral(model) in joint-configurations.hpp, which returns the equivalent of zero for the joint manifold.

In my opinion, we should change one of these names. Both of these could be used for either of the purposes.

I would recommend changing neutralConfiguration to something else.

[jcarpent]

We already discussed this point with @jmirabel and we decided to modify the neuralConfiguration. I would be more to remove it completely and let the SRDF parser returns the value as a ConfigVector type.

[proyan]

I would be okay with this change. However, we still need to give a name to what used to be neutralConfiguration for loading with srdf. If you are not saving it in model anymore, I would recommend defaultConfiguration as the replacement.

[jcarpent]

I would prefer referenceConfigurationVectorbut this one should not be in Model class.

[proyan]

I like that name.

I'll put neutralConfiguration and the srdf function to deprecated, and create a PR with the new names.

[jcarpent]

OK. Sounds good to me!

[nmansard]

If not added to model, where do you want to put it?

[jcarpent]

I think this information should not be inside model because you may have various referenceConfigurationVector. But we may provide just a function to load it from a SRDF file.

[jmirabel]

For me, referenceConfigurationVector means there are several configurations. I don't think Vector adds anything. I would name it referenceConfiguration(s) depending on the number of reference configuration.

If there are several configurations, I think I would store them into a map in order to assign a name to each.

Having a function to extract it from a SRDF is not convenient. It forces developers to store somewhere either the filename or the configurations. In most places where you want this configuration, you don't have the filename.

[proyan]

I think srdf files are meant to be customizable by the developers. So saving this map name->configuration in an srdf file, and making this file available in the robot-description should not be a problem. As developers, we are storing the urdf filenames as well when we load a robot. i don't see why srdf should be different.

[gabrielebndn]

Right now, getNeutralConfiguration specifically looks for a group_state whose name is half_sitting. This name is hard-coded in the function and cannot be changed. It does not make a lot of sense. I guess a more generic name should have been chosen, but the best way would be not to hard-code the name and have it passed as a function input instead. This would automatically open to the possibility of having multiple "neutral configurations", which would make the choice of keeping a single one inside the model quite arbitrary.

In other words: SRDF offers the possibility of defining several reference vectors, I do not see why Pinocchio should not be able to parse all of them. Also, I do not think they belong to the model, because they are essentially not a property of the model, but a choice made by whoever is using the robot.

I think there should be a function that returns a configuration whose name is passed as input, and possibly another function that given an SRDF file returns a name->configuration map.

I do agree that Vector in referenceConfigurationVector doesn't add much.

[jcarpent]

@proyan Can you take in charge the requested modifications?

[proyan]

I am not sure if we all agree on the same modifications. Are you okay if I implement the suggestion of @gabrielebndn?

[jcarpent]

I think we all agree that neutralConfigurationis now deprecated, and should be replaced by something much more generic like referenceConfigurations. I think the idea of @jmirabel of having a map<string,ConfigVectorType> is nice and very flexible.

Then, the loading of a reference configuration vector from a SRDF file should take as input the name of the reference configuration inside the SRDF file + the name of the configuration to store inside the new map.

[gabrielebndn]

@jcarpent so you would keep the map<string,ConfigVectorType> as a variable inside the Model class?

[pFernbach]

Related to https://github.com/humanoid-path-planner/hpp-pinocchio/issues/83 in hpp-pinocchio. I am also in favor of a map name->configuration and a method that initialize this map with all the group_state tags defined in the srdf.

[jcarpent]

@pFernbach It sounds reasonable.

[jcarpent]

@jcarpent so you would keep the map<string,ConfigVectorType> as a variable inside the Model class?

@gabrielebndn Sorry for the late reply, but currently this is the only option I see.

[proyan]

Ok. I'll implement this change.

[jcarpent]

Solved by #666