dqrobotics / cpp

The DQ Robotics library in C++
https://dqrobotics.github.io
GNU Lesser General Public License v3.0
39 stars 13 forks source link

About the DQ_Geometry::point_to_line/plane_(squared)_distance() function #59

Open zhaojiawei392 opened 1 year ago

zhaojiawei392 commented 1 year ago

Professor Murilo, every time I use these two functions in conjunction with DQ_Kinematics::line/plane_topoint(squared)_distance, I always have to remind myself to switch the positions of the point and the line/plane. During such moments, I can't help but think how convenient it would be if there were functions named line_to_point_squared_distance and plane_to_point_distance. I am wondering if you have time and are interested in adding these two delegation functions to improve coding efficiency and readability. :)

bvadorno commented 1 year ago

Dear @zhaojiawei392,

Thank you for your suggestion. If you want to continue this issue, please adapt it to our current standards. The information you gave us needs to be more comprehensive. More specifically:

Describe the missing/unexpected functionality

A clear and concise description of what is happening.

MAKE A MINIMAL EXAMPLE TO SHOW WHAT YOU WANT TO SHOW; THE LONGER YOUR CODE IS, THE LESS LIKELY YOU ARE TO GET PROPER SUPPORT

Current C++ code to achieve the behavior you want

// Your C++ code here

Desired C++ code to achieve the behavior you want

// Your C++ code here

Many thanks, Bruno

zhaojiawei392 commented 1 year ago

Dear Professor @bvadorno,

Thank you for your kind guidance.

The missing/unexpected functionality

zhaojiawei392 commented 1 year ago

Dear Professor @bvadorno,

Thank you for your kind guidance.

The missing/unexpected functionality

Usually the function pair <DQ_Kinematics::line_to_point_distance_jacobian() and 
DQ_Geometry::point_to_line_squared_distance()>or the function pair
 <DQ_Kinematics::plane_to_point_distance_jacobian() and DQ_Geometry::point_to_plane_distance()>
are used together. But their names are not compatible, nor are their arguments. 
It would be more intuitive and easier to have the same naming style(  xx_to_yy ).

Current C++ code

MatrixXd W = DQ_Kinematics::line_to_point_distance_jacobian(J_line, line, point);  // line_to_point: arg1 line, arg2 point
VectorXd w(1);
w << DQ_Geometer::point_to_line_squared_distance(point, line); // point_to_line:  arg0 point, arg1 line
                                                      //need to be careful about the order of the arguments

or 

MatrixXd W = DQ_Kinematics::plane_to_point_distance_jacobian(J_plane, point);
VectorXd w(1);
w << DQ_Geometer::point_to_plane_squared_distance(point, plane);

Desired C++ code

MatrixXd W = DQ_Kinematics::line_to_point_distance_jacobian(J_line, line, point);  // line_to_point: arg1 line, arg2 point
VectorXd w(1);
w << DQ_Geometer::line_to_point_squared_distance(line, point); // line_to_point:  arg0 line, arg1 point

or 

MatrixXd W = DQ_Kinematics::plane_to_point_distance_jacobian(J_plane, point);
VectorXd w(1);
w << DQ_Geometer::plane_to_point_squared_distance(plane, point);

Thank you very much. Jiawei Zhao

mmmarinho commented 1 year ago

@zhaojiawei392 thanks for this suggestion.

I understand where your suggestion is coming from, but first let us discuss this with a bit more depth after I can make a more implementation-oriented summary of what you meant to say.

I think there’s a nice suggestion here that wouldn’t take too much for us to implement, but sometimes these things are not so straightforward in all languages.

mmmarinho commented 1 year ago

@zhaojiawei392

If I understand correctly, this would be an example of what you're proposing. So that the distance functions also have a parallel, even though they are mathematically the same.

DQ_Geometry::point_to_line_squared_distance();
//DQ_Geometry::line_to_point_squared_distance();
DQ_Kinematics::plane_to_point_distance_jacobian();
DQ_Kinematics::point_to_plane_distance_jacobian();

This would be a good first issue for someone to propose such changes in DQRobotics.

Things to do

First

For example: https://github.com/dqrobotics/cpp/blob/77acf9a42875ffb69e9f48f98f3950f9d7242c0e/include/dqrobotics/utils/DQ_Geometry.h#L36

Must have an additional static method such as

static double line_to_point_squared_distance(const DQ& line, const DQ& point)
{
return point_to_line_squared_distance(point,line);
}

Second

Add the new methods to the Python wrapper

https://github.com/dqrobotics/python/blob/de1742ae2eef3220298f2218d98d28031ea0fe1e/src/utils/DQ_Geometry_py.cpp#L33

Third

Add tests for these new methods at https://github.com/dqrobotics/python-tests