Skip to content
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-39796: Update task config defaults to LSST values #253

Merged
merged 1 commit into from Sep 21, 2023

Conversation

parejkoj
Copy link
Contributor

No description provided.

@@ -37,7 +37,7 @@ class CatalogCalculationClassificationConfig(CatalogCalculationPluginConfig):
"""Configuration for catalog classification plugin.
"""

fluxRatio = lsst.pex.config.Field(dtype=float, default=.925, optional=True,
fluxRatio = lsst.pex.config.Field(dtype=float, default=.95, optional=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not suggesting this is wrong, but just wanted to note that this prioritizes the value appropriate for a PSF vs. Gaussian flux ratio (as is used in SFM). For cModel vs. PSF, as an example, the canonical value we use is 0.985 (so each obs package needs to have a cmodel.py config override file:
https://github.com/lsst/obs_lsst/blob/main/config/cmodel.py#L27
https://github.com/lsst/obs_subaru/blob/main/config/cmodel.py#L7
https://github.com/lsst/obs_decam/blob/main/config/cmodel.py#L7
etc...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it looks like we have overrides for the various cameras already?

The only places where those cmodel overrides are used are characterizeImage (which shouldn't be doing cmodel measurements anyway, and will be replaced with CalibrateImage, where no cmodel measurements are done), forcedPhotcoadd, and measureCoaddSources. There's nothing instrument-specific about the overrides you mention above, so we should absolutely move those into the task defaults for those two respective tasks and get rid of the above override files. I'm not going to add more to this ticket, but we already know that there's more cleanup like this ticket to be done, and this is a good example of it.

0.95 is the default for calibrate in LSST and HSC.
@parejkoj parejkoj merged commit 4fadbfc into main Sep 21, 2023
2 checks passed
@parejkoj parejkoj deleted the tickets/DM-39796 branch September 21, 2023 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants