DM-11243: Refine PSF-based image selection #136
Conversation
| @@ -309,11 +311,11 @@ def runDataRef(self, dataRef, coordList, makeDataRefList=True, selectDataList=[] | |||
| scatterSize = sigmaMad(starSize - psfSize) | |||
|
|
|||
| valid = True | |||
| if medianE1 > self.config.maxEllipResidual: | |||
| if self.config.maxEllipResidual and medianE1 > self.config.maxEllipResidual: | |||
| self.log.info("Removing visit %s because median e residual too large: %f" % | |||
There was a problem hiding this comment.
Were you able to confirm that medianE1 is the correct comparison variable here? All the plots I've seen from Takashi Hamana have medianE in the label (but that's not a guarantee of what is actually plotted), but I also note that @rearmstr returns medianE in his sample script here:
http://jeeves.astro.princeton.edu/pipermail/hsc_software/all/5853.html
I also wonder if it's worth adding the config value used in the log.info messages (i.e. print explicitly what "too large" is)?
There was a problem hiding this comment.
I'm fairly convinced @rearmstr meant medianE instead of medianE1, and so I've added a commit fixing that up (see the commit message for my reasoning).
I've also added a commit to include the limit when printing the value.
|
You may also want to update the criteria (now at line 278). It's not very specific, so not explicitly wrong as is (so I won't insist!), but clarification wouldn't hurt. |
We may decide not to use a particular threshold, so they should be optional.
PaulPrice
force-pushed
the
tickets/DM-11243
branch
from
ee55ada
to
411116e
Compare
Takashi Hamana recommends a selection involving:
max_scatterSize/medianSize^2 = 0.009
Takashi Hamana recommends:
max_medianE = 0.007
max_scatterSize/medianSize^2 = 0.009
We update the maxEllipResidual to correspond to Hamana-san's
recommendation and remove the maxSizeScatter limit (maxScaledSizeScatter
is already set appropriately).
The selection was on medianE1 instead of medianE, which appears to have been a typo (log message refers to medianE, and there's no reason to have calculated medianE2 unless we meant to use medianE).
It's more helpful to users to log the actual value along with the limit.
PaulPrice
force-pushed
the
tickets/DM-11243
branch
from
411116e
to
39ce203
Compare
No description provided.