Skip to content

Fix AutoPlatepar CALSTARS requirement and misleading success message#853

Merged
dvida merged 3 commits intoprereleasefrom
fix-autopp
Mar 24, 2026
Merged

Fix AutoPlatepar CALSTARS requirement and misleading success message#853
dvida merged 3 commits intoprereleasefrom
fix-autopp

Conversation

@Cybis320
Copy link
Copy Markdown
Contributor

  • Auto-generate CALSTARS file when missing (matching SkyFit2 behavior)
  • Return None instead of unvalidated intermediate platepar when star matching fails, preventing misleading "SUCCESS" after "ERROR"

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates AutoPlatepar.autoFitPlatepar() to better match SkyFit2 behavior by auto-generating a CALSTARS file when one is missing, and by returning None (instead of an intermediate/unvalidated platepar) when star matching/final fit fails to avoid misleading “SUCCESS” outcomes.

Changes:

  • Auto-generate CALSTARS via extractStarsAndSave when no CALSTARS file exists in the directory.
  • Adjust verbose output to distinguish between loaded vs generated CALSTARS.
  • Change several failure returns to return None for the platepar (and currently None for matched stars) rather than returning an intermediate result.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

I've replaced the brute-force matchStars in CyFunctions.pyx with a scipy.spatial.cKDTree implementation. The new version lives in RMS/Astrometry/MatchStars.py and is a drop-in replacement with the same signature and return format.

@dvida
Copy link
Copy Markdown
Contributor

dvida commented Mar 19, 2026

Is the new approach really faster? Building KD trees takes time and by definition querying is faster only when done multiple times. Here, the KD tree is built for each run.

@Cybis320
Copy link
Copy Markdown
Contributor Author

Is the new approach really faster? Building KD trees takes time and by definition querying is faster only when done multiple times. Here, the KD tree is built for each run.

 ┌───────────────────────┬──────────┬──────────┬─────────────────────┐
 │       Scenario        │  Cython  │ KD-tree  │       Winner        │
 ├───────────────────────┼──────────┼──────────┼─────────────────────┤
 │ Small (20×50)         │ 0.002 ms │ 0.035 ms │ Cython 18x faster   │
 ├───────────────────────┼──────────┼──────────┼─────────────────────┤
 │ Typical (200×500)     │ 0.147 ms │ 0.120 ms │ ~tie                │
 ├───────────────────────┼──────────┼──────────┼─────────────────────┤
 │ Large (500×2000)      │ 1.54 ms  │ 0.46 ms  │ KD-tree 3.3x faster │
 ├───────────────────────┼──────────┼──────────┼─────────────────────┤
 │ Dave 360 (2000×10000) │ 28.9 ms  │ 2.9 ms   │ KD-tree 10x faster  │
 └───────────────────────┴──────────┴──────────┴─────────────────────┘

Cython wins for very small inputs (20×50), that's 20 detected stars x 50 catalog stars, but those are 0.002ms - nobody will notice, and that's not a typical scenario. At typical RMS sizes it's a wash, and it scales much better for larger inputs.

matchStars works in pixel space, so the pixel positions change every time and the KD-tree has to be rebuilt for each frame.

@dvida dvida merged commit 7cfaae6 into prerelease Mar 24, 2026
@dvida dvida deleted the fix-autopp branch April 21, 2026 00:20
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.

3 participants