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

Update bundle adjustment #49

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nmayorov
Copy link
Contributor

@nmayorov nmayorov commented May 4, 2022

I have done some improvements on this notebook:

  1. Clarify definition of camera parameters
  2. Add a comment about relation between images and cameras for this particular setup
  3. Slightly reworked the core part
  4. Replace self-written function for rotation transform with Rotation class

Link to the updated notebook file

https://github.com/scipy/scipy-cookbook/blob/9e136daac9df507db7bf114bf319e6ade9723c21/ipython/bundle_adjustment.ipynb

Fixes #44

@nmayorov
Copy link
Contributor Author

nmayorov commented May 4, 2022

@rgommers

If you have time / interest check this too, I believe it is a clear improvement and safe to merge.

But let's wait for the people from the issues to comment.

@hxu296
Copy link

hxu296 commented Jul 7, 2023

That looks great! I was translating this notebook from scipy to PyTorch and the original Rodrigues vector to unit quaternion implementation introduced some numeric instabiliity that took me a while to hunt down. Your change will solve this issue.

The code that introduced numeric instability:

theta = np.linalg.norm(camera_params[:, :3], axis=1)[:, np.newaxis]
with np.errstate(invalid='ignore'):
    v = camera_params[:, :3] / theta
    v = np.nan_to_num(v)

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.

Unclear variable names in Bundle adjustment notebook
2 participants