DM-39796: Update task config defaults to LSST values #350
Conversation
| @@ -33,7 +33,7 @@ class FindCosmicRaysConfig(pexConfig.Config): | |||
| nCrPixelMax = pexConfig.Field( | |||
| dtype=int, | |||
| doc="maximum number of contaminated pixels", | |||
| default=10000, | |||
| default=1000000, | |||
There was a problem hiding this comment.
Can you explain this change? I see that this is the obs_subaru value, and that obs_decam also used a higher value, so I can see the underlying logic in relation to this ticket, but this value must be too large. For LSSTCam, this amounts to 1 CR pixel per 4x4 grid. HSC and DECam have double the contaminated area. I can't imagine any image with that level of contamination being scientifically useful. I'll dig through the obs_subaru git history to find the reason for the high value, but I just want to raise this issue.
There was a problem hiding this comment.
The large value was also set in obs_lsst, at the root-level characterizeImage override, so it's the defacto default now anyway (modulo the small DECam difference). I'd love to have some rationale for why it is what it is, and whether to make it something better. The goal of this ticket is to just make overrides that are defacto defaults into actual defaults.
There was a problem hiding this comment.
I've added https://jira.lsstcorp.org/browse/DM-40586 as sorting out the correct value is out of scope here.
parejkoj
force-pushed
the
tickets/DM-39796
branch
from
f1a88ac
to
bd5e26a
Compare
| @@ -123,6 +123,7 @@ def test_significance(self): | |||
|
|
|||
| schema = afwTable.SourceTable.makeMinimalSchema() | |||
| config = SourceDetectionTask.ConfigClass() | |||
| config.thresholdType = "stdev" | |||
There was a problem hiding this comment.
Note why you are overriding this for the test.
There was a problem hiding this comment.
The docstring for the test itself explains why.
This was the override for multiple instruments, and is the behavior we generally want by default. Cleanup unnecessary test override defaults
This is the instFlux field most often used in characterize/calibrate, across multiple instruments.
This was the override for multiple instruments.
This was overriden in obs_lsst for calibrate and characterize, but we almost certainly want it everywhere. The tests were written with the original stdev default in mind.
This has not been slow since circa 2015 (see DM-1128 and DM-2787), and is the default for multiple instruments. Also fix the docstring and remove redundant optional kwarg.
These were the defaults for lsst, subaru, and decam.
This isn't always the psfFlux; it depends on the configured flux field.
parejkoj
force-pushed
the
tickets/DM-39796
branch
from
bd5e26a
to
995c10a
Compare
No description provided.