New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DM-13671 add option to skip rank update #80
Conversation
2223299
to
4e396a9
Compare
8dc4db2
to
af25548
Compare
b21e418
to
9a4dad6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Only comment is pre-existing.
python/lsst/jointcal/jointcal.py
Outdated
"""Run fit.minimize up to max_steps times, returning the final chi2.""" | ||
|
||
for i in range(max_steps): | ||
r = fit.minimize(whatToFit, 5) # outlier removal at 5 sigma. | ||
# outlier removal at 5 sigma. | ||
r = fit.minimize(whatToFit, 5, doRankUpdate=doRankUpdate) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pre-existing, but the sigma threshold for outlier rejection should be configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I've been meaning to do that for a while. I've added config.outlierRejectSigma
in a separate commit.
Add jointcalConfig option to skip photometry and astrometry rank update. Add FitterBase support for skipping rank update. Add decam photometry and astrometry tests with and without rank update. Decam photometry doRankUpdate test doesn't work, and is part of the set of "differs on macOS" tests, so we're skipping it for now. Lift out Hessian creation into anonymous function. NOTE: astrometryDoRankUpdate is mostly for testing: it shouldn't have any measureable impact on the astrometry fitter, but it's helpful to demonstrate that FitterBase.minimize() is behaving sanely when we doRankUpdate=False (which is the point of the astrometry skip_rank_update test).
ff8f9b3
to
8723d00
Compare
No description provided.