-
Notifications
You must be signed in to change notification settings - Fork 37
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
update selfcal #430
update selfcal #430
Conversation
rubin_sim/maf/run_selfcal_metric.py
Outdated
@@ -61,7 +72,7 @@ def run_selfcal_metric(): | |||
|
|||
# Make plots of the residuals map | |||
map_bundle = MetricBundle( | |||
IdentityMetric(metric_name="PhotoCal Uniformity"), | |||
IdentityMetric(metric_name="PhotoCal Uniformity, %s" % args.filter), |
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.
Just to make things print nicer in the end, can you drop the ","?
I admit this is just a matter of personal taste but "Photocal Uniformity r <>" (and presumably maybe other bands ..) just reads a bit easier than "Photocal Uniformity, r <>" ...
I guess I don't care a ton but none of the other summary metrics use ","
rubin_sim/maf/run_selfcal_metric.py
Outdated
sql += " and filter='%s'" % args.filter | ||
# Exclude strange exposure times | ||
sql += " and visitExposureTime > 20" | ||
sql += " and visitExposureTime < 40" |
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.
Do we actually need to drop these? It's true it makes things neater, but I would have thought including these exposures wouldn't make the self-calibration worse, and these visits ought to be calibrated as well ..
I'd also wonder about including the DDF visits, tbh, but I think they were excluded just for calculation speed perhaps.
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 was just being extra conservative, but they should be able to be used. That will let in the twilight observations, but I think you're right that they will be weighted properly.
|
||
def run(self, data_slice, slice_point=None): | ||
filter_name = data_slice["filter"][0] |
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.
Well, that's interesting and I think I hadn't noticed. I don't think Eli had specified the constraint on the data either ...
So, ha. Definitely should recheck the more downtime / less downtime outputs.
I also wonder how well the metric tracks against the actual photometric calculation that Eli was expecting to do given that FGCM works across all filters (I think).
97d8f7f
to
f742a5a
Compare
Update
run_selfcal_metric
fornote
toscheduler_note
schema update.General updates to selfcal so filter is set properly.