-
Notifications
You must be signed in to change notification settings - Fork 1
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-43077: Convert all tasks to use CalibrateImageTask outputs #47
base: main
Are you sure you want to change the base?
Conversation
e7ef711
to
efe7d7d
Compare
Update test values to reflect the changed outputs. The image and background mean and std values went up by a factor of ~2, because the CalibrateImage output (`initial_pvi`) is photometrically calibrated, and the fitted calibration is ~2.18.
9ead273
to
107ca63
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.
I'm just confused by the test reference values having to change so much.
|
||
# Confirm only calibrate task uses PropertySet. |
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.
Why was this the case anyway? If nobody remembers, oh well.
("summary.psfIxx", summary.psfIxx, 4.272794013403168, 5.1e-7), | ||
("summary.psfIyy", summary.psfIyy, 4.735316824053334, standard_atol), | ||
("summary.psfIxy", summary.psfIxy, -0.57899030354606, standard_atol), | ||
("im_mean", im_mean, 9.573747386158793, standard_atol), |
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.
These first four are huge changes... what happened?
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.
See my note in the commit message (seemed like the best place to put that): these images are calibrated to nJy, with a calibration factor of ~2.
("summary.dec", summary.dec, -0.23498074547048764, standard_atol), | ||
("summary.zenithDistance", summary.zenithDistance, 21.045745454754197, standard_atol), | ||
("summary.zeroPoint", summary.zeroPoint, 31.4, standard_atol), | ||
("summary.skyBg", summary.skyBg, 392.27323201298714, standard_atol), |
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.
Same here, why did skyBg more than double?
def test_initial_stars(self): | ||
initial_stars = self.butler.get("initial_stars_detector", | ||
detector=self.detector, visit=self.visit) | ||
self.assertEqual(len(initial_stars), 446) |
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.
Higher background and fewer stars is the right sign, I suppose, but again, quite a large change.
No description provided.