Refactor fundamental matrix scaling by matthew-brett · Pull Request #7767 · scikit-image/scikit-image

matthew-brett · 2025-04-04T10:37:25Z

Refactor scaling of FundamentalMatrixTransform calculation to make algorithm clearer, and allow original Hartley algorithm if preferred.

Add some tests of scaling algorithms, including cross-check against opencv2.

Also see https://github.com/matthew-brett/test-fundamental-matrices

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

In `skimage.transform.FundamentalMatrixTransform`, refactor scaling calculation
to make algorithm clearer, and allow original Hartley algorithm if preferred.

Refactor scaling of FundamentalMatrixTransform calculation to make algorithm clearer, and allow original Hartley algorithm if preferred. Add some tests of scaling algorithms, including cross-check against opencv2. Also see <https://github.com/matthew-brett/test-fundamental-matrices>

stefanv · 2025-04-22T17:11:54Z

Discussed on the community call:

The calculation involves using the SVD to examine the null-space of A.
Because of noise, we take the vector corresponding to the smallest singular value.
Hartley & Zisserman explain that scaling, so all elements of A are roughly of the same magnitude, is important for conditioning. The exact method of scaling is likely less important.
Matthew's tests confirm this, and that our current method works fine.
However, this is not the method described by H&Z exactly, so we will provide (somewhat concealed) access to the H+Z method, in case others want to compare for themselves.

* origin/main: Deprecate estimate method in favor of class constructor (scikit-image#7771) Temporary fix for Visual Studio & Clang incompatibility in Windows image (scikit-image#7835) Address deprecations in Pillow 11.3 (scikit-image#7828) Remove unused & obsolete `legacy_datasets`, `legacy_registry` vars (scikit-image#7677) Draft migration guide for skimage2 (scikit-image#7785) Do not report failure in wheels sub-recipe (scikit-image#7806) Use consistent wording in property description. (scikit-image#7804) Add intensity_median to regionprops (scikit-image#7745) CI: Update pypa/gh-action-pypi-publish to v1.12.4 for attestations on PyPI (scikit-image#7793) Document output dtype for transform.resize. (scikit-image#7792) Use `cibuildwheel` to build WASM/Pyodide wheels for `scikit-image`, push nightlies to Anaconda.org (scikit-image#7440) DOC: Include missing gain parameter in adjust_gamma equation (scikit-image#7763) Temporarily pin to `pyodide-build==0.30.0`, and ensure that the correct xbuildenvs are used (scikit-image#7788) Deprecate old names & attributes in RegionProperties (scikit-image#7778) Pin JasonEtco/create-an-issue action to SHA for v2.9.2 (scikit-image#7787) Make doctest-plus work with spin (scikit-image#7786) Report failures on main via issue (scikit-image#7752) Use `workers` instead of alternate parameter names (scikit-image#7302) Fix f-string in otsu plot (scikit-image#7780) Further document use of regionprops function and fix terms. (scikit-image#7518)

mkcor

❤️

skimage/transform/_geometric.py

skimage/transform/tests/test_geometric.py

skimage/transform/_geometric.py

@mkcor

From @mkcor suggestion.

@mkcor

Add various suggestions from @mkcor's review. Co-authored-by: Marianne Corvellec <[email protected]>

@mkcor

rather than the generic link to the paper. From @mkcor's review.

matthew-brett

Responses, with thanks.

skimage/transform/_geometric.py

skimage/transform/tests/test_geometric.py

Use `from_estimate` instead of `estimate`.

matthew-brett · 2025-07-09T14:25:59Z

@mkcor - I've added nearly all your changes with thanks, left one trivial one unmerged - happy either way - let me know.

mkcor

You're most welcome, @matthew-brett. We're almost there!

skimage/transform/_geometric.py

skimage/transform/tests/test_geometric.py

skimage/transform/_geometric.py

skimage/transform/tests/test_geometric.py

@mkcor

More edits from @mkcor. Co-authored-by: Marianne Corvellec <[email protected]>

mkcor

@matthew-brett thanks for bearing with me! Mostly, thank you for undertaking this 'fix' -- I'm glad you folks were able to come up with this compromise. I'll just wait for CI to pass before merging.

* origin/main: Update import convention in certain gallery examples (scikit-image#7764) Refactor fundamental matrix scaling (scikit-image#7767)

* origin/main: (31 commits) Update import convention in certain gallery examples (scikit-image#7764) Refactor fundamental matrix scaling (scikit-image#7767) Add unit test for cval unequal to zero Forward in _generic_edge_filter Remove superfluous mask argument from _generic_edge_filter Only report failure on main branch once Deprecate estimate method in favor of class constructor (scikit-image#7771) Temporary fix for Visual Studio & Clang incompatibility in Windows image (scikit-image#7835) Address deprecations in Pillow 11.3 (scikit-image#7828) Remove unused & obsolete `legacy_datasets`, `legacy_registry` vars (scikit-image#7677) Draft migration guide for skimage2 (scikit-image#7785) Do not report failure in wheels sub-recipe (scikit-image#7806) Use consistent wording in property description. (scikit-image#7804) Add intensity_median to regionprops (scikit-image#7745) CI: Update pypa/gh-action-pypi-publish to v1.12.4 for attestations on PyPI (scikit-image#7793) Document output dtype for transform.resize. (scikit-image#7792) Use `cibuildwheel` to build WASM/Pyodide wheels for `scikit-image`, push nightlies to Anaconda.org (scikit-image#7440) DOC: Include missing gain parameter in adjust_gamma equation (scikit-image#7763) Temporarily pin to `pyodide-build==0.30.0`, and ensure that the correct xbuildenvs are used (scikit-image#7788) Deprecate old names & attributes in RegionProperties (scikit-image#7778) ...

lagru · 2025-11-29T10:23:31Z

Note, I've relabeled this as "maintenance". The new scaling attribute added with this PR isn't really documented. I think it was discussed as a nice to have for people looking at the internal implementation? So let's not advertise a feature in the release notes that's not well documented.

Instead we should document it in a follow-up PR if we want it as public API?

matthew-brett force-pushed the refactor-fundamental branch 3 times, most recently from 7ff66d8 to 5feab11 Compare April 4, 2025 10:48

matthew-brett added the 👶 type: New feature label Apr 4, 2025

matthew-brett requested a review from mkcor April 4, 2025 11:08

matthew-brett force-pushed the refactor-fundamental branch from 5feab11 to 211c7fd Compare April 4, 2025 15:01

Add links to papers.

7e711e7

matthew-brett force-pushed the refactor-fundamental branch from 2086dba to 7e711e7 Compare April 22, 2025 23:45

lagru self-requested a review July 1, 2025 18:17

stefanv approved these changes Jul 7, 2025

View reviewed changes

mkcor reviewed Jul 7, 2025

View reviewed changes

matthew-brett and others added 4 commits July 7, 2025 23:36

Fix docstring link

5fdef11

From @mkcor suggestion.

Apply suggestions from code review

baac2a7

Add various suggestions from @mkcor's review. Co-authored-by: Marianne Corvellec <[email protected]>

Link directly to PDF of paper

3e4b241

rather than the generic link to the paper. From @mkcor's review.

Add back comment removed in refactor.

17f5214

matthew-brett commented Jul 9, 2025

View reviewed changes

skimage/transform/_geometric.py Outdated Show resolved Hide resolved

skimage/transform/tests/test_geometric.py Outdated Show resolved Hide resolved

Fix tests for from_estimate merge

05da87e

Use `from_estimate` instead of `estimate`.

mkcor reviewed Jul 9, 2025

View reviewed changes

matthew-brett and others added 2 commits July 9, 2025 16:32

Apply suggestions from code review

650b4ae

More edits from @mkcor. Co-authored-by: Marianne Corvellec <[email protected]>

Rework a couple of comments.

289faca

mkcor approved these changes Jul 9, 2025

View reviewed changes

mkcor merged commit 61ac6dd into scikit-image:main Jul 9, 2025
24 of 25 checks passed

stefanv added this to the 0.26 milestone Jul 9, 2025

lagru removed the 👶 type: New feature label Nov 29, 2025

lagru added ⏩ type: Enhancement Improve existing features 🔧 type: Maintenance Refactoring and maintenance of internals and removed ⏩ type: Enhancement Improve existing features labels Nov 29, 2025

Uh oh!

Conversation

matthew-brett commented Apr 4, 2025 • edited by lagru Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release note

Uh oh!

stefanv commented Apr 22, 2025 • edited by matthew-brett Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkcor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthew-brett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

matthew-brett commented Jul 9, 2025

Uh oh!

mkcor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkcor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lagru commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matthew-brett commented Apr 4, 2025 •

edited by lagru

Loading

stefanv commented Apr 22, 2025 •

edited by matthew-brett

Loading

lagru commented Nov 29, 2025 •

edited

Loading