Replace MatchedRef(Chi/Diff)Context with pexConfig.ChoiceField

taranu · taranu · commit ffadae84fb81 · 2023-04-06T15:04:34.000-07:00
diff --git a/pipelines/coaddDiffMatchedQualityExtended.yaml b/pipelines/coaddDiffMatchedQualityExtended.yaml
@@ -8,53 +8,10 @@ tasks:
 
       # plots
       plots.matchedRefCModelMagDiffPlot: MatchedRefCoaddCModelFluxPlot
-      plots.matchedRefCModelMagDiffPlot.applyContext: MatchedRefDiffContext
+      plots.matchedRefCModelMagDiffPlot.context: "diff"
 
       plots.matchedRefCModelFluxChiPlot: MatchedRefCoaddCModelFluxPlot
-      plots.matchedRefCModelFluxChiPlot.applyContext: MatchedRefChiContext
-
-      # TODO: Can this be a one liner?
-      plots.matchedRefPositionXDiffPlot: MatchedRefCoaddPositionPlot
-      # TODO: variable must be defined before applyContext; can this be enforced?
-      # (the resulting error if not is not very informative)
-      plots.matchedRefPositionXDiffPlot.variable: x
-      plots.matchedRefPositionXDiffPlot.applyContext: MatchedRefDiffContext
-
-      plots.matchedRefPositionXChiPlot: MatchedRefCoaddPositionPlot
-      plots.matchedRefPositionXChiPlot.variable: x
-      plots.matchedRefPositionXChiPlot.applyContext: MatchedRefChiContext
-
-      plots.matchedRefPositionYDiffPlot: MatchedRefCoaddPositionPlot
-      plots.matchedRefPositionYDiffPlot.variable: y
-      plots.matchedRefPositionYDiffPlot.applyContext: MatchedRefDiffContext
-
-      plots.matchedRefPositionYChiPlot: MatchedRefCoaddPositionPlot
-      plots.matchedRefPositionYChiPlot.variable: y
-      plots.matchedRefPositionYChiPlot.applyContext: MatchedRefChiContext
-
-      # metrics
-      metrics.matchedRefCModelMagDiffMetric: MatchedRefCoaddCModelFluxMetric
-      metrics.matchedRefCModelMagDiffMetric.applyContext: MatchedRefDiffContext
-
-      metrics.matchedRefCModelFluxChiMetric: MatchedRefCoaddCModelFluxMetric
-      metrics.matchedRefCModelFluxChiMetric.applyContext: MatchedRefChiContext
-
-      metrics.matchedRefPositionXDiffMetric: MatchedRefCoaddPositionMetric
-      metrics.matchedRefPositionXDiffMetric.variable: x
-      metrics.matchedRefPositionXDiffMetric.applyContext: MatchedRefChiContext
-
-      metrics.matchedRefPositionXChiMetric: MatchedRefCoaddPositionMetric
-      metrics.matchedRefPositionXChiMetric.variable: x
-      metrics.matchedRefPositionXChiMetric.applyContext: MatchedRefChiContext
-
-      metrics.matchedRefPositionYDiffMetric: MatchedRefCoaddPositionMetric
-      metrics.matchedRefPositionYDiffMetric.variable: y
-      metrics.matchedRefPositionYDiffMetric.applyContext: MatchedRefDiffContext
-
-      metrics.matchedRefPositionYChiMetric: MatchedRefCoaddPositionMetric
-      metrics.matchedRefPositionYChiMetric.variable: y
-      metrics.matchedRefPositionYChiMetric.applyContext: MatchedRefChiContext
-
+      plots.matchedRefCModelFluxChiPlot.context: "chi"
 
       python: |
         from lsst.analysis.tools.analysisMetrics.diffMatchedMetrics import (
@@ -66,8 +23,3 @@ tasks:
           MatchedRefCoaddCModelFluxPlot,
           MatchedRefCoaddPositionPlot,
         )
-
-        from lsst.analysis.tools.contexts._contexts import (
-          MatchedRefDiffContext,
-          MatchedRefChiContext,
-        )
diff --git a/python/lsst/analysis/tools/analysisMetrics/diffMatchedMetrics.py b/python/lsst/analysis/tools/analysisMetrics/diffMatchedMetrics.py
@@ -54,7 +54,7 @@ def _validate(self):
         if self.name_prefix is None or self.unit is None:
             raise ValueError(
                 f"{self.name_prefix=} and {self.unit=} must not be None;"
-                f" did you forget to set a valid context?"
+                f" did you correctly set {self.context=}"
             )
 
     def configureMetrics(
@@ -144,36 +144,48 @@ def __call__(self, data: KeyedData, **kwargs):
 class MatchedRefCoaddCModelFluxMetric(MatchedRefCoaddDiffMagTool, MatchedRefCoaddMetric):
     """Metric for diffs between reference and CModel coadd mags."""
 
-    def matchedRefDiffContext(self):
-        super().matchedRefDiffContext()
-        self.unit = "mmag"
-        self.name_prefix = "photom_mag_cModelFlux_{name_class}_diff_"
-        self.produce.units = self.configureMetrics()
-
-    def matchedRefChiContext(self):
-        super().matchedRefChiContext()
-        self.unit = ""
-        self.name_prefix = "photom_mag_cModelFlux_{name_class}_chi_"
-        self.produce.units = self.configureMetrics()
+    def _setActions(self):
+        if self.context == "diff":
+            self.unit = "mmag"
+            self.name_prefix = "photom_mag_cModelFlux_{name_class}_diff_"
+            self.produce.units = self.configureMetrics()
+        elif self.context == "chi":
+            self.unit = ""
+            self.name_prefix = "photom_mag_cModelFlux_{name_class}_chi_"
+            self.produce.units = self.configureMetrics()
+        else:
+            raise ValueError(f"Unrecognized {self.context=}")
 
     def setDefaults(self):
         super().setDefaults()
 
+    def validate(self):
+        super().validate()
+        # TODO: This check may be fragile and should move to finalize() ASAP
+        if self.name_prefix is None:
+            self._setActions()
+
 
 class MatchedRefCoaddPositionMetric(MatchedRefCoaddDiffPositionTool, MatchedRefCoaddMetric):
     """Metric for diffs between reference and base coadd centroids."""
 
-    def matchedRefDiffContext(self):
-        super().matchedRefDiffContext()
-        self.unit = "pix"
-        self.name_prefix = f"astrom_{self.variable}_{{name_class}}_diff_"
-        self.produce.units = self.configureMetrics()
-
-    def matchedRefChiContext(self):
-        super().matchedRefChiContext()
-        self.unit = ""
-        self.name_prefix = f"astrom_{self.variable}_{{name_class}}_diff_"
-        self.produce.units = self.configureMetrics()
+    def _setActions(self):
+        if self.context == "diff":
+            self.unit = "pix"
+            self.name_prefix = f"astrom_{self.variable}_{{name_class}}_diff_"
+            self.produce.units = self.configureMetrics()
+        elif self.context == "chi":
+            self.unit = ""
+            self.name_prefix = f"astrom_{self.variable}_{{name_class}}_diff_"
+            self.produce.units = self.configureMetrics()
+        else:
+            raise ValueError(f"Unrecognized {self.context=}")
 
     def setDefaults(self):
         super().setDefaults()
+
+    def validate(self):
+        super().validate()
+        # TODO: This check may be fragile and should move to finalize() ASAP
+        if self.name_prefix is None:
+            self._setActions()
diff --git a/python/lsst/analysis/tools/analysisParts/coaddVisit.py b/python/lsst/analysis/tools/analysisParts/coaddVisit.py
@@ -27,7 +27,7 @@
 class CoaddVisitConfig(Config):
     """Config for tools that can be applied in coadd and visit contexts."""
 
-    context = ChoiceField(
+    context: ChoiceField = ChoiceField(
         doc="The analysis context for this class",
         dtype=str,
         allowed={"coadd": "Coadded images", "visit": "Single visit images"},
diff --git a/python/lsst/analysis/tools/analysisParts/diffMatched.py b/python/lsst/analysis/tools/analysisParts/diffMatched.py
@@ -22,7 +22,7 @@
 
 __all__ = ("MatchedRefCoaddTool", "MatchedRefCoaddDiffMagTool", "MatchedRefCoaddDiffPositionTool")
 
-from lsst.pex.config import ChoiceField
+from lsst.pex.config import ChoiceField, Config
 
 from ..actions.vector.selectors import GalaxySelector, StarSelector
 from ..actions.vector.vectorActions import (
@@ -37,6 +37,17 @@
 from ..interfaces import AnalysisTool
 
 
+class MatchedCoaddVisitConfig(Config):
+    """Config for tools that can be applied in coadd and visit contexts."""
+
+    context = ChoiceField(
+        doc="The analysis context for this class",
+        dtype=str,
+        allowed={"coadd": "Coadded images", "visit": "Single visit images"},
+        optional=False,
+    )
+
+
 class MatchedRefCoaddTool(AnalysisTool):
     """Base tool for matched-to-reference metrics/plots on coadds.
 
@@ -50,6 +61,13 @@ class MatchedRefCoaddTool(AnalysisTool):
     appropriate candidates (and stores a match_candidate column).
     """
 
+    context = ChoiceField(
+        doc="The type of metric to compute",
+        dtype=str,
+        allowed={"diff": "Measured - ref value", "chi": "(Measured - ref value)/sigma (a.k.a. chi/pull)"},
+        optional=False,
+    )
+
     def setDefaults(self):
         super().setDefaults()
         self.process.buildActions.fluxes_ref = LoadVector(vectorKey="refcat_flux_{band}")
@@ -85,26 +103,32 @@ class MatchedRefCoaddDiffMagTool(MatchedRefCoaddTool):
     The default model flux is cModel.
     """
 
-    def matchedRefDiffContext(self):
-        self.process.buildActions.diff = SubtractVector(
-            actionA=MagColumnNanoJansky(
-                vectorKey=self.process.buildActions.fluxes_meas.vectorKey, returnMillimags=True
-            ),
-            actionB=DivideVector(
-                actionA=self.process.buildActions.mags_ref,
-                # To convert to mmag
-                actionB=ConstantValue(value=1e-3),
-            ),
-        )
-
-    def matchedRefChiContext(self):
-        self.process.buildActions.diff = DivideVector(
-            actionA=SubtractVector(
-                actionA=LoadVector(vectorKey=self.process.buildActions.fluxes_meas.vectorKey),
-                actionB=LoadVector(vectorKey=self.process.buildActions.fluxes_ref.vectorKey),
-            ),
-            actionB=LoadVector(vectorKey=f"{self.process.buildActions.fluxes_meas.vectorKey}Err"),
-        )
+    def _setActions(self):
+        frozen = self.process._frozen
+        if frozen:
+            self.process._frozen = False
+        if self.context == "diff":
+            self.process.buildActions.diff = SubtractVector(
+                actionA=MagColumnNanoJansky(
+                    vectorKey=self.process.buildActions.fluxes_meas.vectorKey, returnMillimags=True
+                ),
+                actionB=DivideVector(
+                    actionA=self.process.buildActions.mags_ref,
+                    # To convert to mmag
+                    actionB=ConstantValue(value=1e-3),
+                ),
+            )
+        elif self.context == "chi":
+            self.process.buildActions.diff = DivideVector(
+                actionA=SubtractVector(
+                    actionA=LoadVector(vectorKey=self.process.buildActions.fluxes_meas.vectorKey),
+                    actionB=LoadVector(vectorKey=self.process.buildActions.fluxes_ref.vectorKey),
+                ),
+                actionB=LoadVector(vectorKey=f"{self.process.buildActions.fluxes_meas.vectorKey}Err"),
+            )
+        else:
+            raise ValueError(f"Unrecognized {self.context=}")
+        self.process._frozen = frozen
 
     def setDefaults(self):
         super().setDefaults()
@@ -120,6 +144,11 @@ def setDefaults(self):
             vectorKey="diff", selector=VectorSelector(vectorKey="starSelector")
         )
 
+    def validate(self):
+        super().validate()
+        if not hasattr(self.process.buildActions, "diff"):
+            self._setActions()
+
 
 class MatchedRefCoaddDiffPositionTool(MatchedRefCoaddTool):
     """Base tool for diffs between reference and measured coadd astrometry."""
@@ -133,30 +162,25 @@ class MatchedRefCoaddDiffPositionTool(MatchedRefCoaddTool):
         optional=False,
     )
 
-    # TODO: Determine if this can be put back into setDefaults w/o this:
-    # lsst.pex.config.config.FieldValidationError:
-    # Field 'process.buildActions.pos_meas.vectorKey' failed validation:
-    # Required value cannot be None
-    def _setPos(self):
+    def _setActions(self):
         self.process.buildActions.pos_meas = LoadVector(vectorKey=self.variable)
         self.process.buildActions.pos_ref = LoadVector(vectorKey=f"refcat_{self.variable}")
 
-    def matchedRefDiffContext(self):
-        self._setPos()
-        self.process.buildActions.diff = SubtractVector(
-            actionA=self.process.buildActions.pos_meas,
-            actionB=self.process.buildActions.pos_ref,
-        )
-
-    def matchedRefChiContext(self):
-        self._setPos()
-        self.process.buildActions.diff = DivideVector(
-            actionA=SubtractVector(
+        if self.context == "diff":
+            self.process.buildActions.diff = SubtractVector(
                 actionA=self.process.buildActions.pos_meas,
                 actionB=self.process.buildActions.pos_ref,
-            ),
-            actionB=LoadVector(vectorKey=f"{self.process.buildActions.pos_meas.vectorKey}Err"),
-        )
+            )
+        elif self.context == "chi":
+            self.process.buildActions.diff = DivideVector(
+                actionA=SubtractVector(
+                    actionA=self.process.buildActions.pos_meas,
+                    actionB=self.process.buildActions.pos_ref,
+                ),
+                actionB=LoadVector(vectorKey=f"{self.process.buildActions.pos_meas.vectorKey}Err"),
+            )
+        else:
+            raise ValueError(f"Unrecognized {self.context=}")
 
     def setDefaults(self):
         super().setDefaults()
@@ -170,3 +194,8 @@ def setDefaults(self):
         self.process.filterActions.yStars = DownselectVector(
             vectorKey="diff", selector=VectorSelector(vectorKey="starSelector")
         )
+
+    def validate(self):
+        super().validate()
+        if not hasattr(self.process.buildActions, "diff"):
+            self._setActions()
diff --git a/python/lsst/analysis/tools/analysisPlots/diffMatchedPlots.py b/python/lsst/analysis/tools/analysisPlots/diffMatchedPlots.py
@@ -35,6 +35,7 @@ def setDefaults(self):
 
         self.produce.plotTypes = ["galaxies", "stars"]
         self.produce.xAxisLabel = "Reference Magnitude (mag)"
+        self.produce.yAxisLabel = ""
 
 
 class MatchedRefCoaddCModelPlot(MatchedRefCoaddPlot):
@@ -65,26 +66,50 @@ def setDefaults(self):
 
 
 class MatchedRefCoaddCModelFluxPlot(MatchedRefCoaddCModelPlot, MatchedRefCoaddDiffMagTool):
-    def matchedRefDiffContext(self):
-        super().matchedRefDiffContext()
-        self.produce.yAxisLabel = "cModel - Reference mmag"
-
-    def matchedRefChiContext(self):
-        super().matchedRefChiContext()
-        self.produce.yAxisLabel = "chi = (cModel - Reference mag)/error"
+    """Matched reference plot of cModel mags"""
+
+    def _setLabels(self):
+        frozen = self.produce._frozen
+        if frozen:
+            self.produce._frozen = False
+        if self.context == "diff":
+            self.produce.yAxisLabel = "cModel - Reference mmag"
+        elif self.context == "chi":
+            self.produce.yAxisLabel = "chi = (cModel - Reference mag)/error"
+        else:
+            raise ValueError(f"Unrecognized {self.context=}")
+        self.produce._frozen = frozen
 
     def setDefaults(self):
         super().setDefaults()
 
+    def validate(self):
+        super().validate()
+        # TODO: This check may be fragile and should move to finalize() ASAP
+        if self.produce.yAxisLabel == "":
+            self._setLabels()
 
-class MatchedRefCoaddPositionPlot(MatchedRefCoaddCModelPlot, MatchedRefCoaddDiffPositionTool):
-    def matchedRefDiffContext(self):
-        super().matchedRefDiffContext()
-        self.produce.yAxisLabel = f"{self.variable} position (pix)"
 
-    def matchedRefChiContext(self):
-        super().matchedRefChiContext()
-        self.produce.yAxisLabel = f"chi = (slot - Reference {self.variable} position)/error"
+class MatchedRefCoaddPositionPlot(MatchedRefCoaddCModelPlot, MatchedRefCoaddDiffPositionTool):
+    """Matched reference plot of slot positions (RA/Dec)"""
+
+    def _setActions(self):
+        frozen = self.produce._frozen
+        if frozen:
+            self.produce._frozen = False
+        if self.context == "diff":
+            self.produce.yAxisLabel = f"{self.variable} position (pix)"
+        elif self.context == "chi":
+            self.produce.yAxisLabel = f"chi = (slot - Reference {self.variable} position)/error"
+        else:
+            raise ValueError(f"Unrecognized {self.context=}")
+        self.produce._frozen = frozen
 
     def setDefaults(self):
         super().setDefaults()
+
+    def validate(self):
+        super().validate()
+        # TODO: This check may be fragile and should move to finalize() ASAP
+        if self.produce.yAxisLabel == "":
+            self._setLabels()
diff --git a/python/lsst/analysis/tools/contexts/_contexts.py b/python/lsst/analysis/tools/contexts/_contexts.py
diff --git a/tests/test_metrics.py b/tests/test_metrics.py
