DM-30444: Raise when template insufficiently covers the calexp #528
Conversation
yalsayyad
force-pushed
the
tickets/DM-30444
branch
2 times, most recently
from
b4d9fda
to
03f30f6
Compare
There was a problem hiding this comment.
This looks fine for now, and I don't have a specific value in mind for requiredTemplateFraction I suggest changing it to 10% simply because 12% may look carefully chosen by a future developer.
More broadly, while I think this is an improvement and sufficient for now, I think we should change it substantially in the future. I suspect we will want to construct something like an nImage from coaddition and select images (or possibly even parts of images) based on the availability of images across the patch.
| # In the meantime, raise to exit and register as a failure | ||
| message = "Insufficient Template Coverage. (%.1f%% < %.1f%%) Not attempting subtraction." % ( | ||
| 100*templateStruct.area/inputs['exposure'].getBBox().getArea(), | ||
| 100*self.config.requiredTemplateFraction) |
There was a problem hiding this comment.
It might be too verbose, but you could include "Set requiredTemplateFraction=0 to force subtraction."
| requiredTemplateFraction = pexConfig.Field( | ||
| dtype=float, default=0.12, | ||
| doc="Do not attempt to run task if template covers less than this fraction of pixels." | ||
| "Setting to 0 will always attempt image subtraction" |
There was a problem hiding this comment.
Consider setting the threshold to 10% and adding a comment that the specific value is somewhat arbitrary. I worry that future developers will give the specific value a lot of weight and cargo-cult it.
There was a problem hiding this comment.
17% is also on the table as the minimum needed to be able to drop this line excluding two visits that fail because they have 12 and 16% template coverage.
There was a problem hiding this comment.
I agree 17% would also be a reasonable default. We tend to get more junk than good data from the edge regions, so it's reasonable to be more aggressive about throwing it out. Please do explain in the docstring where the number is coming from, though.
|
Yeah, thats what I was saying in the comment of the Jira ticket: |
yalsayyad
force-pushed
the
tickets/DM-30444
branch
2 times, most recently
from
137ecbf
to
0920d1d
Compare
A minimum of 3 visits is needed per pixel for a sufficient template. Because of incomplete coverage of the visits (e.g. chip gaps, edges) the nVisitMin should be higher than 3. No claims that 6 is optimal
What is considered sufficient template coverage is controlled by the config parameter requiredTemplateFraction
yalsayyad
force-pushed
the
tickets/DM-30444
branch
from
0920d1d
to
5ffd91c
Compare
No description provided.