Skip to content

Replace FFT with a Nearest Neighbor Ransac approach and refactor SkyFit#762

Merged
dvida merged 235 commits intoprereleasefrom
feature-scikit-image
Mar 10, 2026
Merged

Replace FFT with a Nearest Neighbor Ransac approach and refactor SkyFit#762
dvida merged 235 commits intoprereleasefrom
feature-scikit-image

Conversation

@Cybis320
Copy link
Copy Markdown
Contributor

@Cybis320 Cybis320 commented Dec 2, 2025

Replace deprecated imreg_dft with scikit-image.

@Cybis320 Cybis320 requested a review from markmac99 December 2, 2025 23:46
@Cybis320 Cybis320 marked this pull request as draft December 2, 2025 23:55
Copy link
Copy Markdown
Contributor

@markmac99 markmac99 left a comment

Choose a reason for hiding this comment

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

I'll have to assume the maths is correct as its not an area i'm familiar with. I left a couple of comments in the review.

Comment thread RMS/Astrometry/FFTalign.py Outdated
Comment thread RMS/Astrometry/FFTalign.py Outdated
@Cybis320
Copy link
Copy Markdown
Contributor Author

Cybis320 commented Dec 5, 2025

Ok, I think it works. It's slightly more accurate (although this a coarse tool so it doesn't matter) and it's slightly faster.

@Cybis320 Cybis320 marked this pull request as ready for review December 5, 2025 02:47
@Cybis320 Cybis320 marked this pull request as draft December 5, 2025 03:33
@dvida
Copy link
Copy Markdown
Contributor

dvida commented Dec 5, 2025

Would could also just use RANSAC to reduce complexity: https://scikit-image.org/docs/0.25.x/auto_examples/transform/plot_matching.html
Apparently there are also USAC and MAGSAC that's included in opencv (>v4.0) which are apparently better, but I never used them myself.

@Cybis320
Copy link
Copy Markdown
Contributor Author

Cybis320 commented Dec 5, 2025

Would could also just use RANSAC to reduce complexity: https://scikit-image.org/docs/0.25.x/auto_examples/transform/plot_matching.html Apparently there are also USAC and MAGSAC that's included in opencv (>v4.0) which are apparently better, but I never used them myself.

Great suggestion!! It's so much better. 185x faster, perfectly accurate, and 180 less lines of code.

@markmac99
Copy link
Copy Markdown
Contributor

@Cybis320 I commented this on the latest commit, but ImageIO version 2.33 isn't available for Python 3.7 which is the standard on Buster builds. The last version thats compatible with python 3.7 is 2.31.2. See https://pypi.org/project/ImageIO/#history for more.

  Could not find a version that satisfies the requirement imageio>=2.33 (from -r requirements.txt (line 16)) (from versions: 1.1-linux32, 1.1-linux64, 1.1-osx64, 1.1-win32, 1.1-win64, 1.2-linux32, 1.2-linux64, 1.2-osx64, 1.2-win32, 1.2-win64, 1.3-linux32, 1.3-linux64, 1.3-osx64, 1.3-win32, 1.3-win64, 0.2.1, 0.2.2, 0.2.3, 0.3.0, 0.3.1, 0.3.2, 0.4, 0.4.1, 0.5, 0.5.1, 1.0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 2.0.0, 2.0.1, 2.1.0, 2.1.1, 2.1.2, 2.2.0, 2.3.0, 2.4.0, 2.4.1, 2.5.0, 2.6.0, 2.6.1, 2.8.0, 2.9.0, 2.10.1, 2.10.2, 2.10.3, 2.10.4, 2.10.5, 2.11.0, 2.11.1, 2.12.0, 2.13.0, 2.13.1, 2.13.2, 2.13.3, 2.13.4, 2.13.5, 2.14.0, 2.14.1, 2.15.0, 2.16.0, 2.16.1, 2.16.2, 2.17.0, 2.18.0, 2.19.0, 2.19.1, 2.19.2, 2.19.3, 2.19.5, 2.20.0, 2.21.0, 2.21.1, 2.21.2, 2.21.3, 2.22.0, 2.22.1, 2.22.2, 2.22.3, 2.22.4, 2.23.0, 2.24.0, 2.25.0, 2.25.1, 2.26.0, 2.26.1, 2.27.0, 2.28.0, 2.28.1, 2.29.0, 2.30.0, 2.31.0, 2.31.1, 2.31.2)
No matching distribution found for imageio>=2.33 (from -r requirements.txt (line 16))
(testsk) pi@uk001l:~/source/testing/testsk $ python -V
Python 3.7.3
(testsk) pi@uk001l:~/source/testing/testsk $ cat /etc/os-release
PRETTY_NAME="Raspbian GNU/Linux 10 (buster)"

@markmac99
Copy link
Copy Markdown
Contributor

Latest update resolves the imageio issue, however frustratingly we have a new problem on Python 3.7 - the latest build of bcrypt is only available as a tar.gz file, and to build it requires Rust which is not installed / available for Buster 32bit.

Building wheels for collected packages: bcrypt
  Building wheel for bcrypt (pyproject.toml) ... error
  error: subprocess-exited-with-error
 This package requires Rust >=1.64.0.

To work around that i think we need to install the last-good wheel version on python 3.7

bcrypt==3.2.0 ; python_version <='3.7'
bcrypt ; python_version >='3.8' 

in the requirements. Sigh.

@Cybis320
Copy link
Copy Markdown
Contributor Author

Isn't that a paramiko dependency? We didn't touch that here or in prerelease. Are you running into issues with a new install on Buster?

@Cybis320 Cybis320 changed the title Replace deprecated imreg_dft with scikit-image Replace FFT with a Nearest Neighbor Ransac approach Dec 11, 2025
@markmac99
Copy link
Copy Markdown
Contributor

Quite possibly. I was testing by trying to install the requirements on Buster in a brand new virtualenv, so that i can identify any potential incompatabilities. Existing stations might be ok, as they'd have bcrypt 3.2.0 installed already. So, it should only be a problem if someone has a buster-based system and has to rebuild it to buster. Maybe we just make a note in the README file explaining the potential issue.

Cybis320 and others added 28 commits February 20, 2026 17:52
Fix re-detect to skip missing files and show per-frame progress. Unify Find Best Frame into a single dialog, with Auto Fit using the real image when available and only creating a placeholder when needed.
…r long operations

Eliminate duplicated auto-fit code path by reusing autoFitAstrometryNet for both image-available and placeholder cases. Add runInBackground helper to run astrometry.net solving in a thread, preventing OS unresponsive warnings. Skip placeholder images in star detection. Add missing time import.
…ratios per-star so missing data doesn't artificially shift brightness. Also correct band name comments (G, BP, RP).
@dvida dvida merged commit e0dbe0b into prerelease Mar 10, 2026
dvida added a commit that referenced this pull request Mar 10, 2026
Replace FFT with a Nearest Neighbor Ransac approach and refactor SkyFit
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.

5 participants