Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Issues with setting values of DynamicsState from Python #33

Open
Neotriple opened this issue Nov 28, 2020 · 3 comments · May be fixed by #40
Open

Issues with setting values of DynamicsState from Python #33

Neotriple opened this issue Nov 28, 2020 · 3 comments · May be fixed by #40
Assignees
Labels
enhancement New feature or request

Comments

@Neotriple
Copy link

Neotriple commented Nov 28, 2020

Hi,

I'm attempting to modify DynamicsState values such as the Center of Mass and Linear Momentum directly in python. Python bindings do not currently exist for the functions that are used to modify these parameters (for example, in order to change value of the center of mass, I would call the DynamicsState::centerOfMass() function located here: https://github.com/machines-in-motion/kino_dynamic_opt/blob/master/momentumopt/include/momentumopt/dynopt/DynamicsState.hpp )

In order to get around this, I modified my local version of the repo as follows:

  • I added a setCenterOfMass() function in the above class/struct:

void setCenterOfMass(const Eigen::Vector3d& com) {com_ = com;}

.def("setCoM", &DynamicsState::setCenterOfMass, py::arg("com"))

I am testing the setter functions in Python, specifically in the motion_planner.py file as follows:

dyn_optimizer.dynamicsSequence().dynamics_states[0].setCoM(np.ones(3))

When I run this, although it gets to the c++ function setCenterOfMass() above, and it looks to actually set the com_ variable that belongs to DynamicsState, if I then print out the value of the dynamicsState in the next python line as follows:

print dyn_optimizer.dynamicsSequence().dynamics_states[0] this does not reflect the change I've made. (it will still report to 0)

For reference, the setCenterOfMass() function works fine in C++ (though I realize it would be redundant, since there is already a function for this in C++)

@MaximilienNaveau
Copy link
Contributor

Hi @Neotriple ,

You should create a pull request for this kind of issue.
Please fork this repository and provide a PR.
We can then discuss on how to improve the code from there.

Thanks,

@Neotriple Neotriple reopened this Jan 7, 2021
@Neotriple Neotriple linked a pull request Jan 7, 2021 that will close this issue
3 tasks
@Neotriple
Copy link
Author

Hi @MaximilienNaveau ,

I've created the PR here: #40

I don't think it should be merged since it was in my fork which had a few other edits, but I think it's good enough for a PR. If you feel otherwise (i.e. I should clear out some of the comments/extra code I added to the solver side of the packages, I can create a new PR as you see fit).

@MaximilienNaveau
Copy link
Contributor

I did a review on the PR, as you said it's not mergeable right now but with a little bit of cleaning it viable.

@MaximilienNaveau MaximilienNaveau linked a pull request Jan 8, 2021 that will close this issue
3 tasks
@MaximilienNaveau MaximilienNaveau added the enhancement New feature or request label Jan 8, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants