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-37955: Update measureApCorrTask to use robust median outlier rejection. #309
Conversation
self.used = schema.addField("apcorr_" + name + "_used", type="Flag", | ||
doc="set if source was used in measuring aperture correction") | ||
self.fluxName = name + "_instFlux" | ||
if not schema.extract(self.fluxName): |
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.
if self.fluxName not in schema:
is more idiomatic, and I sure hope it's not slower.
for iteration in range(self.config.numIter): | ||
resid = z - fitValues | ||
# We allow floating-point slop to allow for field estimation | ||
# noise in a completely flat field. |
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.
Does this avoid a divide-by-zero somewhere? I think that means a slightly longer comment here would be useful.
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 don't think so, but I've rephrased the comment to be clear this shows up in tests when the residual field is completely flat and median_abs_deviation doesn't return identically zero in that case.
tests/test_measureApCorr.py
Outdated
with self.assertRaisesRegex(RuntimeError, "only 4 sources remain"): | ||
self.meas_apCorr_task.run(catalog=sourceCat, exposure=self.exposure) | ||
|
||
# With the measurement algorithm declared as something that might fail, should not get an exception |
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.
Something in this sentence seems awry.
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 copied it from another test, but I have rephrased both of them to be clearer.
91a08cd
to
82353a0
Compare
This replaces the default source selector to use all possible (unflagged) stars, and additional checks on the fit degree after outlier rejection.
82353a0
to
4c50365
Compare
This replaces the default source selector to use all possible (unflagged) stars, and additional checks on the fit degree after outlier rejection.