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 state/measurement models and RDE main class - PR4 #744

Merged
merged 9 commits into from
Nov 8, 2023

Conversation

isorrentino
Copy link
Collaborator

@isorrentino isorrentino commented Oct 24, 2023

This branch is rebased on https://github.com/isorrentino/bipedal-locomotion-framework/tree/RDE/PR3. I convert the PR to draft until #743 is merged.

@isorrentino isorrentino self-assigned this Oct 24, 2023
@isorrentino isorrentino marked this pull request as draft October 24, 2023 14:28
@isorrentino isorrentino changed the title Add python bindings for the RobotDynamicsEstimator library - PR4 Add state/measurement models and RDE main class - PR4 Nov 2, 2023
@isorrentino
Copy link
Collaborator Author

Hi @GiulioRomualdi and @S-Dafarra, the PR is now ready for review.

@isorrentino
Copy link
Collaborator Author

I see a memcheck error but if I run valgrind offline I do not see the error. See the output I have offline:

isorrentino@IITICUBLAP263:~/dev/robotology-superbuild/build/src/bipedal-locomotion-framework (master)$ valgrind ./bin/RobotDynamicsEstimationUnitTests 
==22595== Memcheck, a memory error detector
==22595== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==22595== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==22595== Command: ./bin/RobotDynamicsEstimationUnitTests
==22595== 
Randomness seeded to: 645987130
[2023-11-07 17:47:08.240] [thread: 22595] [blf] [info] Unpacking data...
[2023-11-07 17:47:08.302] [thread: 22595] [blf] [info] Size of s: 500x4
[2023-11-07 17:47:08.306] [thread: 22595] [blf] [info] Size of ds: 500x4
[2023-11-07 17:47:08.309] [thread: 22595] [blf] [info] Size of dds: 500x4
[2023-11-07 17:47:08.311] [thread: 22595] [blf] [info] Size of im: 500x4
[2023-11-07 17:47:08.313] [thread: 22595] [blf] [info] Size of expected_tauj: 500x4
[2023-11-07 17:47:08.315] [thread: 22595] [blf] [info] Size of expected_taum: 500x4
[2023-11-07 17:47:08.318] [thread: 22595] [blf] [info] Size of expected_tauF: 500x4
[2023-11-07 17:47:08.339] [thread: 22595] [blf] [info] Size of fts[ft_1]: 500x6
[2023-11-07 17:47:08.342] [thread: 22595] [blf] [info] Size of fts[ft_2]: 500x6
[2023-11-07 17:47:08.346] [thread: 22595] [blf] [info] Size of accs[ft_1_acc]: 500x3
[2023-11-07 17:47:08.347] [thread: 22595] [blf] [info] Size of accs[ft_2_acc]: 500x3
[2023-11-07 17:47:08.351] [thread: 22595] [blf] [info] Size of gyros[ft_1_gyro]: 500x3
[2023-11-07 17:47:08.352] [thread: 22595] [blf] [info] Size of gyros[ft_2_gyro]: 500x3
[2023-11-07 17:47:08.430] [thread: 22595] [blf] [info] Starting estimation...
===============================================================================
All tests passed (4018 assertions in 1 test case)

==22595== 
==22595== HEAP SUMMARY:
==22595==     in use at exit: 51,292 bytes in 263 blocks
==22595==   total heap usage: 1,275,387 allocs, 1,275,124 frees, 445,230,829 bytes allocated
==22595== 
==22595== LEAK SUMMARY:
==22595==    definitely lost: 0 bytes in 0 blocks
==22595==    indirectly lost: 0 bytes in 0 blocks
==22595==      possibly lost: 0 bytes in 0 blocks
==22595==    still reachable: 49,276 bytes in 242 blocks
==22595==         suppressed: 0 bytes in 0 blocks
==22595== Rerun with --leak-check=full to see details of leaked memory
==22595== 
==22595== For lists of detected and suppressed errors, rerun with: -s
==22595== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

The code is compiled in Debug mode on my laptop.

cc @GiulioRomualdi

@isorrentino
Copy link
Collaborator Author

The error was about the time. The test was taking too long. I reduced the number of samples.

Comment on lines 44 to 48
/**
* Private implementation
*/
// struct Impl;
// std::unique_ptr<Impl> m_pimpl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you removed pimpl you can delete these lines

Comment on lines 59 to 67
// /**
// * Constructor.
// */
// UkfMeasurement();

// /**
// * Destructor.
// */
// virtual ~UkfMeasurement();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove these lines

Comment on lines 97 to 100
*
* UkfMeasurement.ini
*
* dynamics_list ("JOINT_VELOCITY", "MOTOR_CURRENT", "RIGHT_LEG_FT", "RIGHT_FOOT_REAR_GYRO")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*
* UkfMeasurement.ini
*
* dynamics_list ("JOINT_VELOCITY", "MOTOR_CURRENT", "RIGHT_LEG_FT", "RIGHT_FOOT_REAR_GYRO")
* \code{.ini}
* # UkfMeasurement.ini
*
* dynamics_list ("JOINT_VELOCITY", "MOTOR_CURRENT", "RIGHT_LEG_FT", "RIGHT_FOOT_REAR_GYRO")

* covariance (8.2e-8, 1e-2, 9.3e-3)
* dynamic_model "ConstantMeasurementModel"
*
* ~~~~~
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* ~~~~~
* \endcode

* @brief getMeasurementVariableHandler access the `System::VariablesHandler` instance created during the initialization phase.
* @return the measurement variable handler containing all the measurement variables and their sizes and offsets.
*/
System::VariablesHandler& getMeasurementVariableHandler();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can make this function const? returning a const ref

Comment on lines 341 to 344
Eigen::Ref<Eigen::MatrixXd> RDE::UkfState::getInitialStateCovarianceMatrix()
{
return m_initialCovariance;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it can become const

Comment on lines 20 to 33
set(CUSTOM_MODEL_EXPECTED_MD5 33b903fe87939e2e4932564e45741047)
if (EXISTS "${CMAKE_CURRENT_BINARY_DIR}/model.urdf")
file(MD5 "${CMAKE_CURRENT_BINARY_DIR}/model.urdf" CUSTOM_MODEL_CHECKSUM_VARIABLE)
string(COMPARE EQUAL ${CUSTOM_MODEL_CHECKSUM_VARIABLE} ${CUSTOM_MODEL_EXPECTED_MD5} CUSTOM_MODEL_UPDATED)
else()
set(CUSTOM_MODEL_UPDATED FALSE)
endif()

add_bipedal_test(NAME SubModelCreator
SOURCES SubModelCreatorTest.cpp
LINKS BipedalLocomotion::RobotDynamicsEstimator
BipedalLocomotion::ParametersHandler
BipedalLocomotion::ParametersHandlerYarpImplementation
icub-models::icub-models)
endif()
if(NOT ${CUSTOM_MODEL_UPDATED})
message(STATUS "Fetching CUSTOM model from github.com/ami-iit/paper_sorrentino_2024_icra_robot-dynamics-estimation/...")
file(DOWNLOAD https://raw.githubusercontent.com/ami-iit/paper_sorrentino_2024_icra_robot-dynamics-estimation/577c9d734ff0cb7128274b3880106da11678538c/urdf/four_joints_two_sensors.urdf
${CMAKE_CURRENT_BINARY_DIR}/model.urdf
EXPECTED_MD5 ${CUSTOM_MODEL_EXPECTED_MD5})
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should do this only if test is enabled

sampling_time 0.01

[include MODEL "model.ini"]

[include UKF "ukf.ini"]
[include UKF "ukf.ini"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[include UKF "ukf.ini"]
[include UKF "ukf.ini"]


covariance (1e-2, 1e-2, 1e-2, 1e-2, 1e-2, 1e-2)
initial_covariance (1e-2, 1e-2, 1e-2, 1e-2, 1e-2, 1e-2)
dynamic_model "ZeroVelocityStateDynamics"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dynamic_model "ZeroVelocityStateDynamics"
dynamic_model "ZeroVelocityStateDynamics"

@@ -40,6 +40,7 @@ TEST_CASE("MANNTrajectoryGenerator")

iDynTree::ModelLoader ml;
REQUIRE(ml.loadReducedModelFromFile(getRobotModelPath(), jointsList));
BipedalLocomotion::log()->info("{}", getRobotModelPath());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not required

@isorrentino
Copy link
Collaborator Author

@GiulioRomualdi Changes applied.

@GiulioRomualdi GiulioRomualdi enabled auto-merge (squash) November 8, 2023 15:13
@GiulioRomualdi GiulioRomualdi merged commit 944bd60 into ami-iit:master Nov 8, 2023
12 checks passed
@isorrentino isorrentino deleted the RDE/PR4 branch November 8, 2023 15:31
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