Remove dead user_fixed_scale save/restore in SkyFit2 acquisition paths - #960
Open
GlassOnTin wants to merge 1 commit into
Open
Conversation
tryQuickAlignment and getInitialParamsAstrometryNet save and restore self.fixed_scale around their work, but never modify it in between and never forward it into their fitAstrometry(use_nn_cost=True) calls -- so the pair is dead code that reads like unfinished "Fixed scale" wiring. Not forwarding it is correct, not a bug: both methods (and the AutoPlatepar NN callers) fit from scratch with first_platepar_fit=True, i.e. initial acquisition, where the scale must be free to be determined. Pinning it there would degrade acquisition when the box is checked. The "Fixed scale" checkbox is honored where it belongs -- refinement via fitPickedStars, which already passes fixed_scale=self.fixed_scale. Remove the dangling save/restore so the code matches the intent. No behavior change (fixed_scale was never used in these blocks). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to @dvida's note on #951 — the NN auto-recalibration callers save/restore
user_fixed_scalebut never forward it, so the "Fixed scale" checkbox has no effect on automatic recalibration.What I found (all four flagged sites)
SkyFit2.tryQuickAlignmentandSkyFit2.getInitialParamsAstrometryNetsaveself.fixed_scale → user_fixed_scaleand restore it, but never modifyself.fixed_scalein between (they do legitimately force + restoredistortion_type→radial5-odd,refraction,equal_aspect, etc.), and theirfitAstrometry(use_nn_cost=True)calls don't pass it.AutoPlatepar.py:600/693fit withfirst_platepar_fit=Trueand have nofixed_scalehandling at all.All four are
first_platepar_fit=True— initial acquisition, where the scale must be free to be determined. Forwardingfixed_scale=Truethere would pin the scale to a rough seed and degrade acquisition. So not honoring the checkbox during acquisition is correct (as you suspected), and theuser_fixed_scalesave/restore is simply dead code that misleadingly reads like unfinished wiring.Change
Remove the dead
user_fixed_scalesave (×2) and restore (×3). No behavior change —self.fixed_scaleis never read or written in these blocks. The checkbox continues to be honored where it applies: refinement viafitPickedStars, which already forwardsfixed_scale=self.fixed_scale.Note
user_fit_only_pointingin the same two methods is saved/restored but likewise never modified — the identical dead pattern. Happy to drop that too, or — if you'd actually prefer the scale held during acquisition when the box is checked — to instead finish the wiring (forward it into these NN calls). Just say which shape you'd like.🤖 Generated with Claude Code