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-49166: Add ComCam overrides for make(PsfMatched|Direct)Warp #608

Merged
merged 3 commits into from
Feb 26, 2025

Conversation

arunkannawadi
Copy link
Member

No description provided.

config.select.maxScaledSizeScatter = 0.014
config.select.maxPsfTraceRadiusDelta = 0.091
config.select.maxPsfApFluxDelta = 0.047
config.select.maxPsfApCorrSigmaScaledDelta = 0.041
Copy link
Member

Choose a reason for hiding this comment

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

Are these duplicated with the makeWarp configs for this instrument, then, rather than shared as you did for comCam?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a bit conflicted about loading the configs of one task to another, since there are configs that don't match up and depending on what overrides end up where, it could break things. Given the current overrides and pending deprecation, it's not an issue now and can homogenize it either way.

# DM-47171: 9.0 (1.8 arcsec) corresponds to the ~98.5% percentile of the
# PSF FWHM distribution in the 2nd and 3rd weeks of ComCam science observations
# selectDeepCoaddVisits is currently set the the default 2.0 arcsec
# makePsfMatchedWarp configs cannot be loaded in the same way in general.
Copy link
Contributor

Choose a reason for hiding this comment

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

This vague inline comment doesn't really add much information. Is the reason that you PLAN to add makePsfMatchedWarp-specific overrides to the obs_lsst/config/comCam/makePsfMatchedWarp.py one day? If you think that's more likely than this value getting changed, then say something like:
# keep this in sync with config.modelPsf.defaultFwhm in makePsfMatchedWarp.py

and you might as well leave in the 3 line explanation of what 9.0 is in that case too.

Also my fault because I'm the one who changed it, but as of December, the 3rd line lies: selectDeepCoaddVisits is currently set to 1.7 arcsec.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, the reason is if someone added a line like config.warpAndPsfMatch.psfMatch.kernel["AL"].kernelSize = 29, loading that here would result in an error. Given that the classes don't obey an inheritance relationship, it would not be a good idea to load makePsfMatchedWarp config file into makeWarp. I will go ahead with the comment about keeping this in sync.

@arunkannawadi arunkannawadi force-pushed the tickets/DM-49166 branch 2 times, most recently from 0b11e60 to 465ee50 Compare February 26, 2025 16:44
@arunkannawadi arunkannawadi merged commit ac4a6d1 into main Feb 26, 2025
3 checks passed
@arunkannawadi arunkannawadi deleted the tickets/DM-49166 branch February 26, 2025 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants