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

Undistort 2D points function #1026

Merged
merged 10 commits into from
Jun 11, 2021
Merged

Undistort 2D points function #1026

merged 10 commits into from
Jun 11, 2021

Conversation

jhacsonmeza
Copy link
Contributor

Description

undistort_points function to compensate for lens distortion a set of 2D points. Radial, tangential, thin prism, and tilt distortion are considered. Additionally inverseTiltProjection function is used for the tilt distortion compensation.

@edgarriba
Copy link
Member

@jhacsonmeza could you add some uni tests? Check the ones e.g. in geometry epipolar

@jhacsonmeza
Copy link
Contributor Author

I've added a unit test test_opencv where I used two distortion coefficients to be compared with the results given by OpenCV.

@edgarriba edgarriba added 1 Priority 1 🚨 High priority feature request New feature or request module: geometry labels May 30, 2021
@dkoguciuk
Copy link
Contributor

Hi @jhacsonmeza ,

As you probably have noticed that some tests failed, so to be able to move forward we need to fix them:

  1. FLAKE8 - can you modify both source files to be aligned with pep8? You can check this by using: flake8 path/to/code/to/check.py
  2. pytest - can you make sure that all tests pass on your machine (i.e. https://github.com/kornia/kornia/pull/1026/checks?check_run_id=2682007271#step:5:19120)? You can check this by using: pytest path/to/code/to/check.py

@edgarriba
Copy link
Member

@jhacsonmeza @dkoguciuk as for the format we don’t have to worry that much any more since we have not the pre-commit bot (update your pr with master for that). As for gradcheck, make sure that any operations doesn’t contain possible zero divisions. Besides, I’d suggest as discussed in slack to provide batched functionality to make easier to compute e.g loss functions

@jhacsonmeza
Copy link
Contributor Author

@dkoguciuk thank you. I'm going to modify the code to be aligned with pep8. I've used pytest to check my unit test, and I'm working in a problem.

@edgarriba edgarriba marked this pull request as draft June 8, 2021 11:03
@edgarriba
Copy link
Member

@jhacsonmeza code format shouldn’t be a problem any more since the precommit-ci will fix fir you every time you push a commit to the branch

def test_opencv(self, device, dtype):
# ----- Test 1: using 5 distortion coefficients
pts = torch.tensor(
[[1028.0374, 788.7520], [1025.1218, 716.8726], [1022.1792, 645.1857]], device=device, dtype=dtype
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[[1028.0374, 788.7520], [1025.1218, 716.8726], [1022.1792, 645.1857]], device=device, dtype=dtype
[[[1028.0374, 788.7520], [1025.1218, 716.8726], [1022.1792, 645.1857]]], device=device, dtype=dtype

# ptsu_expected = cv2.undistortPoints(pts.numpy().reshape(-1,1,2), K.numpy(),
# dist1.numpy(), None, None, K.numpy()).reshape(-1,2)
ptsu_expected = torch.tensor(
[[1030.5992, 790.65533], [1027.3059, 718.10020], [1024.0700, 645.90600]], device=device, dtype=dtype
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[[1030.5992, 790.65533], [1027.3059, 718.10020], [1024.0700, 645.90600]], device=device, dtype=dtype
[[[1030.5992, 790.65533], [1027.3059, 718.10020], [1024.0700, 645.90600]]], device=device, dtype=dtype

assert points.shape == (B, N, 2)

def test_opencv(self, device, dtype):
# ----- Test 1: using 5 distortion coefficients
Copy link
Member

Choose a reason for hiding this comment

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

Also Split in three different tests
test_opencv_five_coeff
test_opencv_all_coeff
test_opencv_stereo

@edgarriba edgarriba marked this pull request as ready for review June 11, 2021 09:02
@edgarriba edgarriba merged commit 9b89349 into kornia:master Jun 11, 2021
edgarriba added a commit to edgarriba/kornia that referenced this pull request Jul 6, 2021
* 2D undistort points function

* Doctumentation and comments added

* Unit test for undistort_points function

* Optional batch inputs for undistortion

* Code aligned with PEP8

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* split opencv_test into three different test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update kornia/geometry/calibration/undistort.py

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Edgar Riba <edgar.riba@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants