Fix cubic root computation in calibrateHandEye#18164
Fix cubic root computation in calibrateHandEye#18164catree wants to merge 1 commit intoopencv:masterfrom
Conversation
| qy = (m02 - m20) / S; | ||
| qz = (m10 - m01) / S; | ||
| } else if ((m00 > m11)&(m00 > m22)) { | ||
| } else if (m00 > m11 && m00 > m22) { |
There was a problem hiding this comment.
See #18115
I just copy/paste this code from the above link. Don't know if & is intended in the original code but it is better to fix that.
8efea31 to
a5537f2
Compare
alalek
left a comment
There was a problem hiding this comment.
Thank you for contribution!
Please take a look on comments below.
| //Eq 15 | ||
| double det = determinant(R); | ||
| R = pow(sign_double(det) / abs(det), 1.0/3.0) * R; | ||
| R = std::cbrt(std::copysign(1, det) / abs(det)) * R; |
There was a problem hiding this comment.
It would be nice to add check to ensure that det != 0 (like fabs(det) < FLT_EPSILON and failure with StsNoConv)
It may be impossible in math theory, but we are working with approximated computations. There are several parts in OpenCV code which can return negative value for stddev (like 1e-6) and have hacks to clamp these values.
It would be nice to "fail fast" and don't process NaNs.
| methods.push_back(CALIB_HAND_EYE_DANIILIDIS); | ||
|
|
||
| for (size_t idx = 0; idx < methods.size(); idx++) { | ||
| Matx33d R_cam2gripper_est; |
There was a problem hiding this comment.
Lets provide more details in case of failures of complex tests:
for (...) {
SCOPED_TRACE(cv::format("method=%d", methods[idx]));
...
… to handle negative values. Improve doc. Add regression test. Use C++11 std::copysign.
a5537f2 to
49b6928
Compare
|
This can be closed since 3.4 branch has been merged into master. @alalek |
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.
fix #17986