Skip to content

calib3d: fix the test fail on Calib3d_SolvePnP.accuracy#9189

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
tomoaki0705:fixCalib3dRandom
Jul 20, 2017
Merged

calib3d: fix the test fail on Calib3d_SolvePnP.accuracy#9189
opencv-pushbot merged 1 commit intoopencv:masterfrom
tomoaki0705:fixCalib3dRandom

Conversation

@tomoaki0705
Copy link
Copy Markdown
Contributor

@tomoaki0705 tomoaki0705 commented Jul 19, 2017

This pullrequest changes

resolves #9187

SOLVEPNP_UPNP = 4, //!< Exhaustive Linearization for Robust Camera Pose and Focal Length Estimation @cite penate2013exhaustive
SOLVEPNP_AP3P = 5 //!< An Efficient Algebraic Solution to the Perspective-Three-Point Problem @cite Ke17
SOLVEPNP_AP3P = 5, //!< An Efficient Algebraic Solution to the Perspective-Three-Point Problem @cite Ke17
SOLVEPNP_MAX_COUNT = 6, //!< Used for count
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove "value" and "comma":

    __SOLVEPNP_MAX_COUNT   //!< Used for count

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you. Fixed.

Point3f pmax = Point3f(10, 10, 10))
{
const Point3f delta = pmax - pmin;
RNG rng(0x99999999); // fix the seed to use "fixed" input 3D points
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe we should not fix point cloud between different test cases. Try to use cv::theRNG() or ts->get_rng() instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you. Fixed

case SOLVEPNP_UPNP:
opoints = std::vector<Point3f>(points.begin(), points.begin()+4);
break;
case SOLVEPNP_AP3P:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SOLVEPNP_AP3P is swapped with SOLVEPNP_UPNP in this switch statement.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I made a mistake. Thank you for reviewing.

@tomoaki0705
Copy link
Copy Markdown
Contributor Author

tomoaki0705 commented Jul 19, 2017

On my local, this fix let many tests pass, except method == 2
give me more time to investigate

@tomoaki0705
Copy link
Copy Markdown
Contributor Author

One more thing.
The test passes if I loosen the eps of SOLVEPNP_P3P from 1.0e-4 to 2.0e-4 (just twice larger)

I think it's reasonable fix, but any opinion ?

@alalek
Copy link
Copy Markdown
Member

alalek commented Jul 19, 2017

I believe that eps increase is fine.

@tomoaki0705 tomoaki0705 changed the title WIP: fix the test error on Calib3d_SolvePnP.accuracy calib3d: fix the test fail on Calib3d_SolvePnP.accuracy Jul 19, 2017
@tomoaki0705
Copy link
Copy Markdown
Contributor Author

I really appreciate your nice review.
I squashed the commits to one. If the build passes, this PR is ready to merge.

  * move array size to enum
  * move array size to member variable
  * loosen the eps of SOLVEPNP_P3P
  * loosen the eps in Calib3d_SolveP3P.accuracy
@alalek
Copy link
Copy Markdown
Member

alalek commented Jul 20, 2017

Thank you! 👍

@opencv-pushbot opencv-pushbot merged commit 46bee83 into opencv:master Jul 20, 2017
@tomoaki0705 tomoaki0705 deleted the fixCalib3dRandom branch July 20, 2017 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calib3d_SolvePnP.accuracy is too unstable based on random seed

3 participants