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

getMaximum2DCurvature wrong curvature calculation #1119

Closed
TauTheLepton opened this issue Oct 24, 2023 · 2 comments
Closed

getMaximum2DCurvature wrong curvature calculation #1119

TauTheLepton opened this issue Oct 24, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@TauTheLepton
Copy link
Contributor

Describe the bug
CatmullRomSpline member function getMaximum2DCurvature calculates the wrong value.
The member function finds the greatest max 2D curvature value from all curves that are included in the spline. This is done in the line below.

return *std::max_element(maximum_2d_curvatures_.begin(), maximum_2d_curvatures_.end());

However the max 2D curvature can be a positive as well as a negative value. The curvature magnitude is determined by the absolute value of the curvature value, which is considered in the function below.
double HermiteCurve::getMaximum2DCurvature() const
{
const auto values = get2DMinMaxCurvatureValue();
if (std::fabs(values.first) > std::fabs(values.second)) {
return values.first;
}
return values.second;
}

This means that the CatmullRomSpline member function getMaximum2DCurvature having two values: -10 and 2 will choose 2, however the value -10 corresponds to a much greater curvature, just in the other direction. For a spline having all component curves bending in one way (when all component max 2D curvatures are negative) this member function actually does the opposite of what is should - it selects the smallest curvature.
The logic present in HermiteCurve member function should be applied to a CatmullRomSpline member function as well.

Context:
I am adding unit tests for the geometry package and wanted to add tests for the getMaximum2DCurvature function, but it generates the wrong result in a case similar to the example above.

To Reproduce
Steps to reproduce the behavior:

  1. Edit any existing source code file or create a new one
  2. Create a CatmullRomSpline passing through points (0, 0), (0.5, 0.5), (1, 0), (2, -1) and (3, 0)
  3. Call the getMaximum2DCurvature member function
  4. Print and see the incorrect result

Expected behavior
Greatest absolute value should be selected instead of the greatest one.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: [e.g. iOS]: Ubuntu
  • Browser [e.g. chrome, safari]: Firefox
  • Version [e.g. 22]: 22
  • ROS 2 version: Humble
  • DDS: CycloneDDS
@hakuturu583
Copy link
Collaborator

Thanks, it is a bug.

@hakuturu583 hakuturu583 added the bug Something isn't working label Oct 27, 2023
@hakuturu583 hakuturu583 self-assigned this Oct 27, 2023
@hakuturu583
Copy link
Collaborator

#1139 was merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants