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

PnP update #64

Merged
merged 17 commits into from
Jul 13, 2020
Merged

PnP update #64

merged 17 commits into from
Jul 13, 2020

Conversation

marip8
Copy link
Collaborator

@marip8 marip8 commented Jun 25, 2020

Per investigation in #57, this PR provides a workaround for using Eigen objects without local paramterization. It also adds a function for calculating the "full" covariance matrix between two parameter blocks (i.e. variance with the parameter block itself and covariance with the other parameter block)

@marip8 marip8 force-pushed the update/pnp_utest branch from 3a36f5e to c201ea5 Compare June 25, 2020 23:13
@marip8 marip8 force-pushed the update/pnp_utest branch from e952c58 to 2bd2e70 Compare June 29, 2020 21:22
@marip8
Copy link
Collaborator Author

marip8 commented Jun 29, 2020

@schornakj can you review? This is ready to merge but has a few changes that might overlap with #62

rct_optimizations/src/rct_optimizations/pnp.cpp Outdated Show resolved Hide resolved
rct_optimizations/src/rct_optimizations/pnp.cpp Outdated Show resolved Hide resolved
rct_optimizations/test/pnp_utest.cpp Outdated Show resolved Hide resolved
@schornakj
Copy link
Contributor

@schornakj can you review? This is ready to merge but has a few changes that might overlap with #62

I'll rebase onto your changes after we merge. #62 adds a function that's similar (but uses different underlying Ceres functions) to the "full" covariance function you added here so you might be interested in taking a look and seeing which approach you prefer.

@marip8
Copy link
Collaborator Author

marip8 commented Jul 10, 2020

@schornakj I addressed your comments in the latest commits. I ended up running the randomly initialized optimizations 30 times and setting expectations that the mean error + 3 standard deviations should be less than a (still somewhat arbitrary) threshold. I think this is at least a step in the right direction for setting reasonable expectations for unit tests with randomness

@schornakj
Copy link
Contributor

@schornakj I addressed your comments in the latest commits. I ended up running the randomly initialized optimizations 30 times and setting expectations that the mean error + 3 standard deviations should be less than a (still somewhat arbitrary) threshold. I think this is at least a step in the right direction for setting reasonable expectations for unit tests with randomness

That's reasonable. Looks good to me!

@marip8 marip8 merged commit 530b7bd into Jmeyer1292:master Jul 13, 2020
@marip8 marip8 deleted the update/pnp_utest branch July 13, 2020 14:49
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.

2 participants