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

unicycle motion model: add option to rotate the process noise covariance to the current state orientation #325

Open
wants to merge 8 commits into
base: devel
Choose a base branch
from
Open
5 changes: 3 additions & 2 deletions fuse_models/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,14 @@ if(CATKIN_ENABLE_TESTING)
roslint_add_test()

# Model tests
catkin_add_gtest(test_unicycle_2d
add_rostest_gtest(
test_unicycle_2d
test/unicycle_2d.test
test/test_unicycle_2d.cpp
)
target_link_libraries(test_unicycle_2d
${PROJECT_NAME}
${catkin_LIBRARIES}
${CERES_LIBRARIES}
)
set_target_properties(test_unicycle_2d
PROPERTIES
Expand Down
2 changes: 1 addition & 1 deletion fuse_models/include/fuse_models/unicycle_2d.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class Unicycle2D : public fuse_core::AsyncMotionModel
*
* @param[in] state1 The first/oldest state
* @param[in] state2 The second/newest state
* @param[in] process_noise_covariance The process noise covariance, after it is scaled and multiplied by dt
* @param[in] process_noise_covariance The process noise covariance, after it is rotated, scaled and multiplied by dt
*/
static void validateMotionModel(const StateHistoryElement& state1, const StateHistoryElement& state2,
const fuse_core::Matrix8d& process_noise_covariance);
Expand Down
13 changes: 10 additions & 3 deletions fuse_models/src/unicycle_2d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ void Unicycle2D::onInit()

private_node_handle_.param("scale_process_noise", scale_process_noise_, scale_process_noise_);
private_node_handle_.param("velocity_norm_min", velocity_norm_min_, velocity_norm_min_);

private_node_handle_.param("disable_checks", disable_checks_, disable_checks_);

double buffer_length = 3.0;
Expand Down Expand Up @@ -348,8 +347,16 @@ void Unicycle2D::generateMotionModel(
state_history_.emplace(beginning_stamp, std::move(state1));
state_history_.emplace(ending_stamp, std::move(state2));

// Scale process noise covariance pose by the norm of the current state twist
auto process_noise_covariance = process_noise_covariance_;

// Rotate the process noise covariance with the yaw angle of the current state orientation
const auto rotation_matrix = Eigen::Rotation2Dd(state1.pose.yaw()).toRotationMatrix();
// Apply only to x and y position as the other state variables are already along the
// current state orientation
process_noise_covariance.topLeftCorner<2, 2>() =
rotation_matrix * process_noise_covariance.topLeftCorner<2, 2>() * rotation_matrix.transpose();
Comment on lines +352 to +357
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little confused with this. The actual residual that is computed for the unicycle constraints is just the position difference. That difference is expressed in the world/map frame here, so rotating the process noise into the frame doesn't make sense to me. This would make sense if that residual is also expressed in the first poses frame.

Copy link
Contributor

Choose a reason for hiding this comment

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

@svwilliams Has your team had a chance to validate or test this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at some point, but haven't had time to actually fix it.
From what I remember, I agree with the assessment that the current covariance is not represented in the same frame as the error. However, I believe the correct thing to do is change the error calculation to be in the frame of Pose1 rather than to spin the covariance to be in the global frame.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that makes more sense to me. I can work on a PR that does that


// Scale process noise covariance pose by the norm of the current state twist
if (scale_process_noise_)
{
common::scaleProcessNoiseCovariance(process_noise_covariance, state1.velocity_linear, state1.velocity_yaw,
Expand All @@ -367,7 +374,7 @@ void Unicycle2D::generateMotionModel(
}
catch (const std::runtime_error& ex)
{
ROS_ERROR_STREAM_THROTTLE(10.0, "Invalid '" << name_ << "' motion model: " << ex.what());
ROS_ERROR_STREAM_THROTTLE(10.0, "Invalid '" << name() << "' motion model: " << ex.what());
return;
}
}
Expand Down
169 changes: 167 additions & 2 deletions fuse_models/test/test_unicycle_2d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
***************************************************************************/
#include <fuse_models/unicycle_2d.h>

#include <fuse_models/unicycle_2d_state_kinematic_constraint.h>
#include <fuse_graphs/hash_graph.h>
#include <fuse_variables/acceleration_linear_2d_stamped.h>
#include <fuse_variables/orientation_2d_stamped.h>
Expand All @@ -15,17 +16,41 @@
#include <ros/time.h>

#include <gtest/gtest.h>
#include <fuse_core/eigen_gtest.h>

#include <string>
#include <vector>

/**
* @brief Derived class used in unit tests to expose protected functions
* @brief Derived class used in unit tests to expose protected/private functions and variables
*/
class Unicycle2DModelTest : public fuse_models::Unicycle2D
{
public:
using fuse_models::Unicycle2D::updateStateHistoryEstimates;
using fuse_models::Unicycle2D::StateHistoryElement;
using fuse_models::Unicycle2D::StateHistory;
using fuse_models::Unicycle2D::generateMotionModel;

void setProcessNoiseCovariance(const fuse_core::Matrix8d& process_noise_covariance)
{
process_noise_covariance_ = process_noise_covariance;
}

void setStateHistory(const StateHistory& state_history)
{
state_history_ = state_history;
}

void setScaleProcessNoise(const bool& scale_process_noise)
{
scale_process_noise_ = scale_process_noise;
}

void setDisableChecks(const bool& disable_checks)
{
disable_checks_ = disable_checks;
}
};

TEST(Unicycle2D, UpdateStateHistoryEstimates)
Expand Down Expand Up @@ -291,8 +316,148 @@ TEST(Unicycle2D, UpdateStateHistoryEstimates)
}
}

TEST(Unicycle2D, generateMotionModel)
{
// Create some variables
auto position1 = fuse_variables::Position2DStamped::make_shared(ros::Time(1, 0));
auto yaw1 = fuse_variables::Orientation2DStamped::make_shared(ros::Time(1, 0));
auto linear_velocity1 = fuse_variables::VelocityLinear2DStamped::make_shared(ros::Time(1, 0));
auto yaw_velocity1 = fuse_variables::VelocityAngular2DStamped::make_shared(ros::Time(1, 0));
auto linear_acceleration1 = fuse_variables::AccelerationLinear2DStamped::make_shared(ros::Time(1, 0));
position1->x() = 1.1;
position1->y() = 2.1;
yaw1->yaw() = 1.2;
linear_velocity1->x() = 1.0;
linear_velocity1->y() = 0.0;
yaw_velocity1->yaw() = 0.0;
linear_acceleration1->x() = 1.0;
linear_acceleration1->y() = 0.0;

const ros::Time beginning_stamp = position1->stamp();
const ros::Time ending_stamp = beginning_stamp + ros::Duration{1.0};

Unicycle2DModelTest::StateHistory state_history;
state_history.emplace(
position1->stamp(),
Unicycle2DModelTest::StateHistoryElement{ // NOLINT(whitespace/braces)
position1->uuid(),
yaw1->uuid(),
linear_velocity1->uuid(),
yaw_velocity1->uuid(),
linear_acceleration1->uuid(),
tf2_2d::Transform(position1->x(), position1->y(), yaw1->yaw()),
tf2_2d::Vector2(linear_velocity1->x(), linear_velocity1->y()),
yaw_velocity1->yaw(),
tf2_2d::Vector2(linear_acceleration1->x(), linear_acceleration1->y())}); // NOLINT(whitespace/braces)

Unicycle2DModelTest unicycle_2d_model_test;

unicycle_2d_model_test.setStateHistory(state_history);

std::vector<double> process_noise_diagonal{1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0};
unicycle_2d_model_test.setProcessNoiseCovariance(fuse_core::Vector8d(process_noise_diagonal.data()).asDiagonal());

unicycle_2d_model_test.setScaleProcessNoise(false);
unicycle_2d_model_test.setDisableChecks(false);

std::vector<fuse_core::Constraint::SharedPtr> constraints;
std::vector<fuse_core::Variable::SharedPtr> variables;

// Generate the motion model
unicycle_2d_model_test.generateMotionModel(
beginning_stamp,
ending_stamp,
constraints,
variables);

// Check first the created variables
{
// check the first entry (the original value before prediction)

auto position = std::static_pointer_cast<fuse_variables::Position2DStamped>(variables.at(0));
auto orientation = std::static_pointer_cast<fuse_variables::Orientation2DStamped>(variables.at(1));
auto velocity_linear = std::static_pointer_cast<fuse_variables::VelocityLinear2DStamped>(variables.at(2));
auto velocity_yaw = std::static_pointer_cast<fuse_variables::VelocityAngular2DStamped>(variables.at(3));
auto acceleration_linear = std::static_pointer_cast<fuse_variables::AccelerationLinear2DStamped>(variables.at(4));

auto expected_pose = tf2_2d::Transform(position1->x(), position1->y(), yaw1->yaw());
auto expected_linear_velocity = tf2_2d::Vector2(linear_velocity1->x(), linear_velocity1->y());
auto expected_yaw_velocity = yaw_velocity1->yaw();
auto expected_linear_acceleration = tf2_2d::Vector2(linear_acceleration1->x(), linear_acceleration1->y());

EXPECT_EQ(position->x(), expected_pose.x());
EXPECT_EQ(position->y(), expected_pose.y());
EXPECT_EQ(orientation->yaw(), expected_pose.yaw());

EXPECT_EQ(velocity_linear->x(), expected_linear_velocity.x());
EXPECT_EQ(velocity_linear->y(), expected_linear_velocity.y());

EXPECT_EQ(velocity_yaw->yaw(), expected_yaw_velocity);

EXPECT_EQ(acceleration_linear->x(), expected_linear_acceleration.x());
EXPECT_EQ(acceleration_linear->y(), expected_linear_acceleration.y());
}

{
// check the second entry (the prediction)

auto position = std::static_pointer_cast<fuse_variables::Position2DStamped>(variables.at(5));
auto orientation = std::static_pointer_cast<fuse_variables::Orientation2DStamped>(variables.at(6));
auto velocity_linear = std::static_pointer_cast<fuse_variables::VelocityLinear2DStamped>(variables.at(7));
auto velocity_yaw = std::static_pointer_cast<fuse_variables::VelocityAngular2DStamped>(variables.at(8));
auto acceleration_linear = std::static_pointer_cast<fuse_variables::AccelerationLinear2DStamped>(variables.at(9));

auto expected_pose = tf2_2d::Transform(1.643536632, 3.498058629, 1.2);
auto expected_linear_velocity = tf2_2d::Vector2(2.0, 0.0);
auto expected_yaw_velocity = 0.0;
auto expected_linear_acceleration = tf2_2d::Vector2(1.0, 0.0);

EXPECT_NEAR(position->x(), expected_pose.x(), 1.0e-9);
EXPECT_NEAR(position->y(), expected_pose.y(), 1.0e-9);
EXPECT_NEAR(orientation->yaw(), expected_pose.yaw(), 1.0e-9);

EXPECT_NEAR(velocity_linear->x(), expected_linear_velocity.x(), 1.0e-9);
EXPECT_NEAR(velocity_linear->y(), expected_linear_velocity.y(), 1.0e-9);

EXPECT_NEAR(velocity_yaw->yaw(), expected_yaw_velocity, 1.0e-9);

EXPECT_NEAR(acceleration_linear->x(), expected_linear_acceleration.x(), 1.0e-9);
EXPECT_NEAR(acceleration_linear->y(), expected_linear_acceleration.y(), 1.0e-9);
}

// Now check the created constraint
{
auto new_constraint =
std::static_pointer_cast<fuse_models::Unicycle2DStateKinematicConstraint>(constraints.back());

// As based on the yaw angle we are mainly moving upwards and the process noise covariance
// is defined in the robot frame (original: cov_xx=1.0, cov_yy=2.0) it is expected that the final
// process noise covariance in the world frame (so the frame where optimization happens) is higher
// at cov_xx than in cov_yy.
// Additionally, cov_xy and cov_yx are also no more zero (so no more independent between x and y) because
// again it is moving simultaneously in the x- and y-direction.
fuse_core::Matrix8d expected_cov;
expected_cov << 1.868696858, -0.33773159, 0, 0, 0, 0, 0, 0,
-0.33773159, 1.131303142, 0, 0, 0, 0, 0, 0,
0, 0, 3, 0, 0, 0, 0, 0,
0, 0, 0, 4, 0, 0, 0, 0,
0, 0, 0, 0, 5, 0, 0, 0,
0, 0, 0, 0, 0, 6, 0, 0,
0, 0, 0, 0, 0, 0, 7, 0,
0, 0, 0, 0, 0, 0, 0, 8;

EXPECT_MATRIX_NEAR(expected_cov, new_constraint->covariance(), 1.0e-9);
}
}

int main(int argc, char **argv)
{
testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
ros::init(argc, argv, "unicycle_2d_test");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need ROS?

Copy link
Contributor Author

@fabianhirmann fabianhirmann May 17, 2023

Choose a reason for hiding this comment

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

We need ROS because the Unicycle2D class is derived from the fuse_core::AsyncMotionModel which contains variables of ros::NodeHandle and ros::AsyncSpinner and if we don't initialize ROS, there are the following errors shown:

[FATAL ${node} ros.roscpp 1684306444.156373179]: You must call ros::init() before creating the first NodeHandle
[ERROR ${node} ros.roscpp 1684306444.167166713]: [registerPublisher] Failed to contact master at [:0].  Retrying...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. Before the unit tests didn't create a Unicycle2DTest object, but now you create one.

I don't think there's any workaround to avoid this, so I think it's fine.

auto spinner = ros::AsyncSpinner(1);
spinner.start();
int ret = RUN_ALL_TESTS();
spinner.stop();
ros::shutdown();
return ret;
}
4 changes: 4 additions & 0 deletions fuse_models/test/unicycle_2d.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0"?>
<launch>
<test test-name="unicycle_2d_test" pkg="fuse_models" type="test_unicycle_2d" />
</launch>