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-29348: Add ability to run global fgcmcal in a single pipeline. #59

Merged
merged 1 commit into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions python/lsst/fgcmcal/fgcmCalibrateTractBase.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def setDefaults(self):
pexConfig.Config.setDefaults(self)

self.fgcmFitCycle.quietMode = True
self.fgcmFitCycle.doPlots = False
self.fgcmOutputProducts.doReferenceCalibration = False
self.fgcmOutputProducts.doRefcatOutput = False
self.fgcmOutputProducts.cycleNumber = 0
Expand Down Expand Up @@ -379,9 +380,6 @@ def run(self, dataRefDict, tract,
True, False, lutIndexVals[0]['FILTERNAMES'],
tract=tract)

# Turn off plotting in tract mode
configDict['doPlots'] = False

# Use the first orientation.
# TODO: DM-21215 will generalize to arbitrary camera orientations
ccdOffsets = computeCcdOffsets(dataRefDict['camera'], fgcmExpInfo['TELROT'][0])
Expand Down
224 changes: 180 additions & 44 deletions python/lsst/fgcmcal/fgcmFitCycle.py

Large diffs are not rendered by default.

38 changes: 13 additions & 25 deletions python/lsst/fgcmcal/fgcmOutputProducts.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,31 +85,31 @@ class FgcmOutputProductsConnections(pipeBase.PipelineTaskConnections,
deferLoad=True,
)

fgcmVisitCatalog = connectionTypes.PrerequisiteInput(
fgcmVisitCatalog = connectionTypes.Input(

Choose a reason for hiding this comment

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

Why are we changing all these to just Inputs? As far as I can tell, some are still always used (unlike eg fgcmAtmosphereParameters that may not be needed depending on the config) here and in fgcmFitCycle...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously, the three tasks (build stars; fit; output products) all had to be done separately. And I believe a PrerequisiteInput is faster for the the quantum graph generator to find (or not find). But now I have the ability to link them all into one pipeline, so the fgcmVisitCatalog doesn't actually exist prior to the start of the pipeline run, so it must be a regular Input that is constrained by the quantum graph generator.

doc="Catalog of visit information for fgcm",
name="fgcmVisitCatalog",
storageClass="Catalog",
dimensions=("instrument",),
deferLoad=True,
)

fgcmStandardStars = connectionTypes.PrerequisiteInput(
fgcmStandardStars = connectionTypes.Input(
doc="Catalog of standard star data from fgcm fit",
name="fgcmStandardStars{cycleNumber}",
storageClass="SimpleCatalog",
dimensions=("instrument",),
deferLoad=True,
)

fgcmZeropoints = connectionTypes.PrerequisiteInput(
fgcmZeropoints = connectionTypes.Input(
doc="Catalog of zeropoints from fgcm fit",
name="fgcmZeropoints{cycleNumber}",
storageClass="Catalog",
dimensions=("instrument",),
deferLoad=True,
)

fgcmAtmosphereParameters = connectionTypes.PrerequisiteInput(
fgcmAtmosphereParameters = connectionTypes.Input(
doc="Catalog of atmosphere parameters from fgcm fit",
name="fgcmAtmosphereParameters{cycleNumber}",
storageClass="Catalog",
Expand All @@ -126,12 +126,6 @@ class FgcmOutputProductsConnections(pipeBase.PipelineTaskConnections,
multiple=True,
)

fgcmBuildStarsTableConfig = connectionTypes.PrerequisiteInput(
doc="Config used to build FGCM input stars",
name="fgcmBuildStarsTable_config",
storageClass="Config",
)

fgcmPhotoCalib = connectionTypes.Output(
doc=("Per-visit photometric calibrations derived from fgcm calibration. "
"These catalogs use detector id for the id and are sorted for "
Expand Down Expand Up @@ -173,9 +167,9 @@ def __init__(self, *, config=None):
if not config.doReferenceCalibration:
self.prerequisiteInputs.remove("refCat")
if not config.doAtmosphereOutput:
self.prerequisiteInputs.remove("fgcmAtmosphereParameters")
self.inputs.remove("fgcmAtmosphereParameters")
if not config.doZeropointOutput:
self.prerequisiteInputs.remove("fgcmZeropoints")
self.inputs.remove("fgcmZeropoints")
if not config.doReferenceCalibration:
self.outputs.remove("fgcmOffsets")

Expand All @@ -189,7 +183,12 @@ class FgcmOutputProductsConfig(pipeBase.PipelineTaskConfig,
dtype=int,
default=None,
)

physicalFilterMap = pexConfig.DictField(
doc="Mapping from 'physicalFilter' to band.",
keytype=str,
itemtype=str,
default={},
)
# The following fields refer to calibrating from a reference
# catalog, but in the future this might need to be expanded
doReferenceCalibration = pexConfig.Field(
Expand Down Expand Up @@ -408,18 +407,7 @@ def runQuantum(self, butlerQC, inputRefs, outputRefs):
else:
self.refObjLoader = None

dataRefDict['fgcmBuildStarsTableConfig'] = butlerQC.get(inputRefs.fgcmBuildStarsTableConfig)

fgcmBuildStarsConfig = butlerQC.get(inputRefs.fgcmBuildStarsTableConfig)
physicalFilterMap = fgcmBuildStarsConfig.physicalFilterMap

if self.config.doComposeWcsJacobian and not fgcmBuildStarsConfig.doApplyWcsJacobian:
raise RuntimeError("Cannot compose the WCS jacobian if it hasn't been applied "
"in fgcmBuildStarsTask.")
if not self.config.doComposeWcsJacobian and fgcmBuildStarsConfig.doApplyWcsJacobian:
self.log.warn("Jacobian was applied in build-stars but doComposeWcsJacobian is not set.")
Comment on lines -413 to -420

Choose a reason for hiding this comment

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

Why were these checks removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We seem to be unable to read in the configs as regular inputs, and these checks seemed extraneous. But my intention is to add these to the gen3 pipeline "contracts". The pipeline itself isn't anywhere yet since it doesn't quite fit into the current framework; I'll ticket that separately.


struct = self.run(dataRefDict, physicalFilterMap, returnCatalogs=True)
struct = self.run(dataRefDict, self.config.physicalFilterMap, returnCatalogs=True)

# Output the photoCalib exposure catalogs
if struct.photoCalibCatalogs is not None:
Expand Down
4 changes: 3 additions & 1 deletion python/lsst/fgcmcal/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"""

import numpy as np
import os
import re

from lsst.daf.base import PropertyList
Expand All @@ -37,7 +38,6 @@
from lsst.obs.base import createInitialSkyWcs
from lsst.obs.base import Instrument


import fgcm


Expand Down Expand Up @@ -203,9 +203,11 @@ def makeConfigDict(config, log, camera, maxIter,
'quietMode': config.quietMode,
'randomSeed': config.randomSeed,
'outputStars': False,
'outputPath': os.path.abspath('.'),
'clobber': True,
'useSedLUT': False,
'resetParameters': resetFitParameters,
'doPlots': config.doPlots,
'outputFgcmcalZpts': True, # when outputting zpts, use fgcmcal format
'outputZeropoints': outputZeropoints}

Expand Down
1 change: 0 additions & 1 deletion tests/config/fgcmBuildStarsHsc.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
config.physicalFilterMap = {'HSC-G': 'g', 'HSC-R': 'r', 'HSC-I': 'i'}
config.requiredBands = ['r', 'i']
config.primaryBands = ['i']
config.minPerBand = 2
Expand Down
6 changes: 0 additions & 6 deletions tests/config/fgcmFitCycleHsc.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
import lsst.fgcmcal as fgcmcal

config.outfileBase = 'TestFgcm'
# The unused z-band is here to test the case that extra filters

Choose a reason for hiding this comment

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

We're still getting that extra-filter test with HSC_FILTER_DEFINITIONS.physical_to_band, right?

# are in the map that are not used in the calibrations.
config.physicalFilterMap = {'HSC-G': 'g',
'HSC-R': 'r',
'HSC-I': 'i',
'HSC-Z': 'z'}
config.bands = ['g', 'r', 'i']
config.fitBands = ['g', 'r', 'i']
config.requiredBands = ['r', 'i']
Expand Down