Geometry Transforms: avoid division by zero & some degenerate cases#3926
Geometry Transforms: avoid division by zero & some degenerate cases#3926stefanv merged 1 commit intoscikit-image:masterfrom
Conversation
9d60ab1 to
9a9297c
Compare
|
Hey, @cv3d, thank you for submitting this PR. Could you please expand on the problem you ran into, and why this is the correct fix? |
|
Hi @stefanv. I am using these geometry transforms in a RANSAC iteration for model estimation, where some degenerative cases might be the cause of such errors. The basic idea of this PR is to return some numerical values (even if they are very large) rather than NaNs, which might break the user code. |
|
@cv3d I think we would like some comments around those lines to explain their purpose, as well as a test case that would fail without the fixes (so that if anyone changes the logic or the algorithm in the future, the new algorithm still has the same stability). |
9a9297c to
b8e89f4
Compare
b8e89f4 to
0414efb
Compare
jni
left a comment
There was a problem hiding this comment.
Great! I just verified that these tests fail on master. Personally I would like more detail. For example,
- in
# avoid division by zero, instead write "below, we will divide by the last dimension of the homogeneous coordinate matrix. In order to avoid division by zero, we replace exact zeros in this column with a very small number." - in
# is it a degenerate case?, for example, "if the last element of the vector corresponding to the smallest singular value is close to zero, this implies a degenerate case because..."
In the tests, you could add notes about why the tests are like this: instead of "either indicate a degenerate case, or return a transform. Prior to gh-3926, under these circumstances, a transform could be returned with nan values."
- avoid division by zero - avoid some degenerate cases
0414efb to
97159d8
Compare
|
@cv3d thanks for the update! Looks great! @scikit-image/core imho this is merge-ready! |
|
Thanks, @cv3d! |
Description
Minor update to avoid returning nans due to division by zeros & some degenerate cases
Checklist
./doc/examples(new features only)./benchmarks, if your changes aren't covered by anexisting benchmark
For reviewers
later.
__init__.py.doc/release/release_dev.rst.@meeseeksdev backport to v0.14.x