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

Implement ICentroidalMPC interface and add the StableCentroidalMPC class #734

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mebbaid
Copy link
Contributor

@mebbaid mebbaid commented Sep 29, 2023

This PR replaces the one in #703.

In this PR I will add the StableCentroidalMPC as derived class from a ICentroidalMPC together with keeping the original CentroidalMPC implementation unchanged.

The idea is also to add the ParametrizedCentroidalMPC derived class. However I will leave that to a separate PR till I fix the associated unit test issue and merge this one.

@mebbaid mebbaid changed the title Feature/stable centroidal mpc Feature/stable and paramtrized centroidal mpc Oct 2, 2023
@mebbaid mebbaid marked this pull request as draft October 2, 2023 16:38
@mebbaid
Copy link
Contributor Author

mebbaid commented Oct 2, 2023

Put the PR back in draft in light of https://github.com/ami-iit/element_walking-with-payload/issues/59

@mebbaid mebbaid changed the title Feature/stable and paramtrized centroidal mpc Feature/stable centroidal mpc Oct 25, 2023
@mebbaid
Copy link
Contributor Author

mebbaid commented Oct 25, 2023

For similar weights, the unit test produces the following results for tracking and angular momentum, which are very satisfactory to me. I will put this PR for review now so we agree on the details.

angular_h_unitTest
com_unitTest

@mebbaid mebbaid marked this pull request as ready for review October 25, 2023 13:18
@mebbaid
Copy link
Contributor Author

mebbaid commented Oct 26, 2023

I am not sure why the valgrind CI check fails. Running the unitTest doing valgrind ./StableCentroidalMPCUnitTests in my machine i get the following output

===============================================================================
All tests passed (255 assertions in 1 test case)

==169640== 
==169640== HEAP SUMMARY:
==169640==     in use at exit: 15,319 bytes in 40 blocks
==169640==   total heap usage: 13,634,673 allocs, 13,634,633 frees, 368,146,061,381 bytes allocated
==169640== 
==169640== LEAK SUMMARY:
==169640==    definitely lost: 0 bytes in 0 blocks
==169640==    indirectly lost: 0 bytes in 0 blocks
==169640==      possibly lost: 0 bytes in 0 blocks
==169640==    still reachable: 15,319 bytes in 40 blocks
==169640==         suppressed: 0 bytes in 0 blocks
==169640== Rerun with --leak-check=full to see details of leaked memory
==169640== 
==169640== For lists of detected and suppressed errors, rerun with: -s
==169640== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

which seems to indicate no memory errors. Any input on the CI failing for the memcheck would be helpful @GiulioRomualdi

@mebbaid
Copy link
Contributor Author

mebbaid commented Oct 30, 2023

As per suggestion, need to reduce the samples for the unit test.

@mebbaid
Copy link
Contributor Author

mebbaid commented Nov 7, 2023

I did some checks to verify that the CI is not failing for something more important in https://github.com/ami-iit/element_walking-with-payload/issues/53. The CI failure is incidental with no actual memory errors and not reproducible in local machines. Additionally changing the linear solver used (similar to what we do on the robot) significantly reduced computational time required.

@mebbaid
Copy link
Contributor Author

mebbaid commented Nov 7, 2023

PR is ready for review @GiulioRomualdi @S-Dafarra

@mebbaid

This comment was marked as outdated.

@mebbaid
Copy link
Contributor Author

mebbaid commented Jan 10, 2024

The CI on ubuntu Debug is failing with

The following tests FAILED:
	112 - memcheck_ArucoDetectorTestUnitTests (Failed)
Errors while running CTest
Error: Process completed with exit code 8

which is not related to this PR i guess @GiulioRomualdi @S-Dafarra

Now this is not failing anymore. and PR ready for review.

@GiulioRomualdi GiulioRomualdi changed the title Feature/stable centroidal mpc Implement ICentroidalMPC interface and add the STableCentroidalMPC class Jan 21, 2024
@GiulioRomualdi GiulioRomualdi changed the title Implement ICentroidalMPC interface and add the STableCentroidalMPC class Implement ICentroidalMPC interface and add the StableCentroidalMPC class Jan 21, 2024
@mebbaid
Copy link
Contributor Author

mebbaid commented Feb 8, 2024

We take the opportunity of putting this PR back in draft to test

@mebbaid mebbaid marked this pull request as draft February 8, 2024 10:34
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.

1 participant