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-16702: Add reference star support to fgcmcal fitting #13
Conversation
This task is designed to be called from third-party code, and will return numpy-formatted catalogs with fgcm-friendly units. All bands are retrieved at the same time.
Also add referenceSelector support.
Previously, when replacing flux values, any redder band that depended on a bluer band would blow up because of the change of units.
export RCRERUN=RC/w_2018_38/DM-15690 | ||
export COOKBOOKRERUN=fgcm_cookbook_w_2018_38 | ||
export RCRERUN=RC/w_2019_18/DM-19151-sfm | ||
export COOKBOOKRERUN=fgcm_cookbook_w_2019_18 | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! Updated doc!
cookbook/README.md
Outdated
you will find a sharded reference catalog suitable to use for any observations | ||
overlapping the survey images that have been calibrated. | ||
|
||
And in the output repo | ||
`/datasets/hsc/repo/rerun/private/erykoff/rc2_w_2018_32/fit1/jointcal-results` | ||
`/datasets/hsc/repo/rerun/private/${USER}/${COOKBOOKRERUN}/fit1/jointcal-results` | ||
you will find all of the `jointcal_photoCalib` spatially-variable zeropoint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this still a jointcal_photoCalib
but above you say it output a "fgcm_photoCalib` files for each visit/ccd"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
cookbook/fgcmBuildStarsHsc.py
Outdated
@@ -14,12 +18,24 @@ | |||
config.densityCutMaxPerPixel = 1500 | |||
# Dictionary that maps "filters" (instrumental configurations) to "bands" | |||
# (abstract names). All filters must be listed in the LUT. | |||
config.filterToBand = {'g':'g', 'r':'r', 'i':'i', 'z':'z', 'y':'y'} | |||
config.filterMap = {'g':'g', 'r':'r', 'i':'i', 'z':'z', 'y':'y'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs spaces after colons.
# Reference star signal-to-noise minimum to use in calibration | ||
config.refStarSnMin = 50.0 | ||
# Number of sigma compared to average mag for reference star to be considered an outlier | ||
config.refStarOutlierNSig = 4.0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
W391: blank line at end of file
@@ -7,7 +7,7 @@ | |||
from lsst.utils import getPackageDir | |||
|
|||
# Do the reference catalog calibration | |||
doReferenceCalibration = True | |||
doReferenceCalibration = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to contract the inline comment right above
# in the reference catalog. This is okay. | ||
pass | ||
|
||
if foundReferenceFilter: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The break
can go right after self._referenceFilter = filterName
@@ -73,7 +73,7 @@ class FgcmOutputProductsConfig(pexConfig.Config): | |||
doReferenceCalibration = pexConfig.Field( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the relationship between this doReferenceCalibration
and the one in fgcmFitCycle?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've improved the doc string: "Transfer 'absolute' calibration from reference catalog? This afterburner step is unnecessary if reference stars were used in the full fit in FgcmFitCycleTask."
I've also added a warning in FgcmOutputProductsTask
if both are set: "doReferenceCalibration is set, and is possibly redundant with fitCycleConfig.doReferenceCalibration". (It's not an error, it's just a warning). Does that make sense?
tests/test_loadref_hsc.py
Outdated
|
||
import lsst.fgcmcal as fgcmcal | ||
|
||
ROOT = os.path.abspath(os.path.dirname(__file__)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable
tests/test_loadref_hsc.py
Outdated
|
||
""" | ||
|
||
import matplotlib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused?
tests/test_loadref_hsc.py
Outdated
import lsst.utils | ||
import lsst.pex.exceptions | ||
import lsst.pipe.tasks | ||
import lsst.daf.persistence as dafPersistence |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dafPersist
is the standard alias.
This PR covers a major upgrade in fgcm that not only adds reference stars to the fit as an option, but has a large number of bug fixes and fitter improvements that lead to much improved calibration on HSC, including combining r/r2 and i/i2 filters. Additional systematic plots are also created, more sophisticated tracking of instrumental changes, etc.