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

Add the possibility to set the name of each element of a variable stored in the variables handler #429

Merged
merged 6 commits into from
Oct 22, 2021

Conversation

GiulioRomualdi
Copy link
Member

As written in the title with this PR it is possible to set the label of each element of a variable stored in the handler. Thanks to this it will be easier to retrieve the index of the variable

The feature is already implemented also for the python bindings

In [1]: import bipedal_locomotion_framework.bindings as blf                                         
In [2]: a = blf.system.VariablesHandler()                                                           
In [3]: a.add_variable("joint_acceleration_with_names", 3, ["torso", "hip", "ankle"])               
In [4]: a.add_variable("joint_acceleration", 10)                                                    

In [5]: print(a)                                                                                    
joint_acceleration size: 10, offset: 3 elements name: joint_acceleration_0 joint_acceleration_1 joint_acceleration_2 joint_acceleration_3 joint_acceleration_4 joint_acceleration_5 joint_acceleration_6 joint_acceleration_7 joint_acceleration_8 joint_acceleration_9joint_acceleration_with_names size: 3, offset: 0 elements name: torso hip ankle

@S-Dafarra
Copy link
Member

S-Dafarra commented Oct 21, 2021

Besides the trivial comments, the code looks good to me. There is one thing I am not entirely convinced of. If you want to have sub-labels, you need to give a label to each sub-element. Also, in this way you can have only one level of sub-labelling. In some cases, it might be useful to have multiple levels. Like

- optimization_variables
                  |- joint_values
                             |- j1
                             |-j2
                  |- base_velocity
                             |- linear_velocity
                                         |-x
                                         |-y
                                         |-z
                             |- angular_velocity

I was wondering if we could use the variable labeller class iteratively. Basically, when adding a variable, you pass directly a VariableLabeller object. Do you see what I mean?

In any case, if you are not convinced, or you think it is not very feasible, we can forget this and move on without issues.

@GiulioRomualdi
Copy link
Member Author

I see what you mean but I don't know how we can handle it

@S-Dafarra
Copy link
Member

Basically in

std::unordered_map<std::string, std::ptrdiff_t> elementsNameMap;

you would have VariablesHandler instead of ptrdiff_t. Then instead of getElementIndex you could have getElement returning a VariableDescription (as with getVariable).

In order to set the sub labels, you would need to pass a VariablesHandler object here

const std::vector<std::string>& elementsName) noexcept;

I am not entirely sure this is possible anyway. Also, when passing the VariablesHandler object you would need to add an offset to all the internal labels.

In any case, this was just to drop an idea. Feel free to ignore this and the comment above.

@GiulioRomualdi
Copy link
Member Author

GiulioRomualdi commented Oct 21, 2021

mmmh at this point I don't know if we should keep separated the variable handler and the variable description classes. I think it would be nice to have something like

auto x = variable.getElement('base_velocity').getElement('linear_velocity).getElement('x')
x.size ///---> 1
x.offset ///----> the offset in the entire variable (in this case is the offset of linear_velocity

auto base_velocity = variable.getElement('base_velocity')

Tomorrow morning I try to figure out something.

By the way just to give you the reason why I opened this PR because I would like to implement a task in TSID where we can regularize a variable to a given quantity passed to the user. Here the variable could be the joint torque but I don't want to regularize all of them but only 4 to a given quantity.
With this PR it was easy to retrieve the index of the specific variable in the entire optimization problem

@GiulioRomualdi
Copy link
Member Author

By the way we should be carefull in the case a new variable is added in an already existing group. All the next existing variables offset must be shifted of the size of the new variable and the group containing that variable must be expanded

@S-Dafarra
Copy link
Member

S-Dafarra commented Oct 22, 2021

By the way we should be carefull in the case a new variable is added in an already existing group. All the next existing variables offset must be shifted of the size of the new variable and the group containing that variable must be expanded

A possibility could be also to just store the offset of the first element, and you add this offset everytime you want to retrieve an element. Hence, with

auto x = variable.getElement('base_velocity').getElement('linear_velocity`).getElement('x')

base_velocity has offset, let's say, 20. Hence, getElement('base_velocity') returns an object which already stores 20 inside. Then, linear_velocity has offset 3. Then, the returned object has a base offset of 20 + 3 = 23. Finally x has offset 0, so the final offset is 23.

Inside VariableDescriptor we could have a pointer to the containing VariableHandler. Let's call it handler. Then, inside VariableHandler we could have a VariableDescriptor* parent{nullptr} parameter. Finally, we would have a getBaseIndex method in VariableHandler like

std::std::ptrdiff_t VariableHandler::getBaseIndex() const
{
    if (parent == 0) return 0;
    if (parent->handler == 0) return -1; //The descriptor is not part of any handler
    return parent->offset + parent->handler->getBaseIndex();
}

By the way just to give you the reason why I opened this PR because I would like to implement a task in TSID where we can regularize a variable to a given quantity passed to the user. Here the variable could be the joint torque but I don't want to regularize all of them but only 4 to a given quantity.
With this PR it was easy to retrieve the index of the specific variable in the entire optimization problem

It seems a good idea! If what I have proposed so far is not helping for the use case you need, feel free to move on 😉

@GiulioRomualdi
Copy link
Member Author

If it is fine with you I can handle the suggestion you proposed here: #429 (comment) and #429 (comment) and merge the PR as it is. Then I can open an issue to keep track of this idea and we may implement it later on since right now I don;t have a clear idea on how to do it in a clean and proper way.

@S-Dafarra
Copy link
Member

If it is fine with you I can handle the suggestion you proposed here: #429 (comment) and #429 (comment) and merge the PR as it is. Then I can open an issue to keep track of this idea and we may implement it later on since right now I don;t have a clear idea on how to do it in a clean and proper way.

Ok no problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants