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.

Sign up for GitHub

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

Jump to bottom

DM-30869: Modernize MetricTask for better Gen 3 workflow #105

Merged
merged 9 commits into from
Sep 20, 2022
Merged

DM-30869: Modernize MetricTask for better Gen 3 workflow #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8552604
Remove requirement that MetricTask.run accept None values.
kfindeisen Sep 12, 2022
51aee9d
Remove None support from MetadataMetricTask.
kfindeisen Sep 12, 2022
7cd35b9
Remove None support from ApdbMetricTask.
kfindeisen Sep 12, 2022
147f9af
Remove requirement that MetricTask.run return None.
kfindeisen Sep 14, 2022
64f0c82
Fix broken links to MetricComputationError.
kfindeisen Sep 14, 2022
f77bfab
Use NoWorkFound in TimingMetricTask and MemoryMetricTask.
kfindeisen Sep 14, 2022
cb0c69d
Use NoWorkFound in ApdbMetricTask.
kfindeisen Sep 14, 2022
7ab305d
Remove exception handling from ApdbMetricTask.runQuantum.
kfindeisen Sep 16, 2022
dd782a8
Remove exception handling from MetricTask.runQuantum.
kfindeisen Sep 16, 2022
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
10 changes: 5 additions & 5 deletions doc/lsst.verify/tasks/lsst.verify.tasks.MetricTask.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,17 @@ Error Handling
In general, a ``MetricTask`` may run in three cases:

#. the task can compute the metric without incident.
#. the task does not have the datasets required to compute the metric.
This often happens if the user runs generic metric configurations on arbitrary pipelines, or if they make changes to the pipeline configuration that enable or disable processing steps.
More rarely, it can happen when trying to compute diagnostic metrics on incomplete (i.e., failed) pipeline runs.
#. the task does not have the data required to compute the metric.
This can happen with metadata- or table-based metrics if the user runs generic metric configurations on arbitrary pipelines, or if they make changes to the pipeline configuration that enable or disable processing steps.
Middleware automatically handles the case where an entire dataset is missing.
#. the task has the data it needs, but cannot compute the metric.
This could be because the data are corrupted, because the selected algorithm fails, or because the metric is ill-defined given the data.

A ``MetricTask`` must distinguish between these cases so that calling frameworks can handle them appropriately.
A task for a metric that does not apply to a particular pipeline run (case 2) must return `None` in place of a `~lsst.verify.Measurement`.
A task for a metric that does not apply to a particular pipeline run (case 2) must either raise `~lsst.pipe.base.NoWorkFound` or return `None`; it must not return a dummy value or raise a different exception.
A task that cannot give a valid result (case 3) must raise `~lsst.verify.tasks.MetricComputationError`.

In grey areas, developers should choose a ``MetricTask``'s behavior based on whether the root cause is closer to case 2 or case 3.
For example, :lsst-task:`~lsst.verify.tasks.commonMetrics.TimingMetricTask` accepts top-level task metadata as input, but returns `None` if it can't find metadata for the subtask it is supposed to time.
While the input dataset is available, the subtask metadata are most likely missing because the subtask was never run, making the situation equivalent to case 2.
The subtask metadata are most likely missing because the subtask was never run, making the situation equivalent to case 2.
On the other hand, metadata with nonsense values falls squarely under case 3.
74 changes: 34 additions & 40 deletions python/lsst/verify/tasks/apdbMetricTask.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@

from lsst.pex.config import Config, ConfigurableField, ConfigurableInstance, \
ConfigDictField, ConfigChoiceField, FieldValidationError
from lsst.pipe.base import Task, Struct, connectionTypes
from lsst.pipe.base import NoWorkFound, Task, Struct, connectionTypes
from lsst.dax.apdb import make_apdb, ApdbConfig

from lsst.verify.tasks import MetricTask, MetricConfig, MetricConnections, \
MetricComputationError
from lsst.verify.tasks import MetricTask, MetricConfig, MetricConnections


class ConfigApdbLoader(Task):
Expand All @@ -54,7 +53,7 @@ def _getApdb(self, config):

Parameters
----------
config : `lsst.pex.config.Config` or `None`
config : `lsst.pex.config.Config`
A config that may contain a `lsst.dax.apdb.ApdbConfig`.
Behavior is undefined if there is more than one such member.

Expand All @@ -64,8 +63,6 @@ def _getApdb(self, config):
A `lsst.dax.apdb.Apdb` object or a drop-in replacement, or `None`
if no `lsst.dax.apdb.ApdbConfig` is present in ``config``.
"""
if config is None:
return None
if isinstance(config, ApdbConfig):
return make_apdb(config)

Expand Down Expand Up @@ -100,7 +97,7 @@ def _getApdbFromConfigurableField(self, configurable):

Parameters
----------
configurable : `lsst.pex.config.ConfigurableInstance` or `None`
configurable : `lsst.pex.config.ConfigurableInstance`
A configurable that may contain a `lsst.dax.apdb.ApdbConfig`.

Returns
Expand All @@ -109,9 +106,6 @@ def _getApdbFromConfigurableField(self, configurable):
A `lsst.dax.apdb.Apdb` object or a drop-in replacement, if a
suitable config exists.
"""
if configurable is None:
return None

if issubclass(configurable.ConfigClass, ApdbConfig):
return configurable.apply()
else:
Expand All @@ -122,7 +116,7 @@ def _getApdbFromConfigIterable(self, configDict):

Parameters
----------
configDict: iterable of `lsst.pex.config.Config` or `None`
configDict: iterable of `lsst.pex.config.Config`
A config iterable that may contain a `lsst.dax.apdb.ApdbConfig`.

Returns
Expand All @@ -131,19 +125,17 @@ def _getApdbFromConfigIterable(self, configDict):
A `lsst.dax.apdb.Apdb` object or a drop-in replacement, if a
suitable config exists.
"""
if configDict:
for config in configDict:
result = self._getApdb(config)
if result:
return result
return None
for config in configDict:
result = self._getApdb(config)
if result:
return result

def run(self, config):
"""Create a database consistent with a science task config.

Parameters
----------
config : `lsst.pex.config.Config` or `None`
config : `lsst.pex.config.Config`
A config that should contain a `lsst.dax.apdb.ApdbConfig`.
Behavior is undefined if there is more than one such member.

Expand Down Expand Up @@ -181,7 +173,7 @@ def run(self, config):

Parameters
----------
config : `lsst.dax.apdb.ApdbConfig` or `None`
config : `lsst.dax.apdb.ApdbConfig`
A config for the database connection.

Returns
Expand Down Expand Up @@ -219,6 +211,7 @@ class ApdbMetricConnections(
"by AP processing.",
storageClass="Config",
multiple=True,
minimum=1,
dimensions={"instrument", "visit", "detector"},
)
# Replaces MetricConnections.measurement, which is detector-level
Expand Down Expand Up @@ -289,9 +282,13 @@ def makeMeasurement(self, dbHandle, outputDataId):

Raises
------
MetricComputationError
lsst.verify.tasks.MetricComputationError
Raised if an algorithmic or system error prevents calculation of
the metric. See `run` for expected behavior.
lsst.pipe.base.NoWorkFound
Raised if the metric is ill-defined or otherwise inapplicable to
the database state. Typically this means that the pipeline step or
option being measured was not run.
"""

def run(self, dbInfo, outputDataId={}):
Expand All @@ -318,42 +315,39 @@ def run(self, dbInfo, outputDataId={}):

Raises
------
MetricComputationError
lsst.verify.tasks.MetricComputationError
Raised if an algorithmic or system error prevents calculation of
the metric.
lsst.pipe.base.NoWorkFound
Raised if the metric is ill-defined or otherwise inapplicable to
the database state. Typically this means that the pipeline step or
option being measured was not run.

Notes
-----
This implementation calls
`~lsst.verify.tasks.ApdbMetricConfig.dbLoader` to acquire a database
handle (taking `None` if no input), then passes it and the value of
handle, then passes it and the value of
``outputDataId`` to `makeMeasurement`. The result of `makeMeasurement`
is returned to the caller.
"""
db = self.dbLoader.run(dbInfo[0] if dbInfo else None).apdb
db = self.dbLoader.run(dbInfo[0]).apdb

if db is not None:
measurement = self.makeMeasurement(db, outputDataId)
return Struct(measurement=self.makeMeasurement(db, outputDataId))
else:
measurement = None

return Struct(measurement=measurement)
raise NoWorkFound("No APDB to measure!")

def runQuantum(self, butlerQC, inputRefs, outputRefs):
"""Do Butler I/O to provide in-memory objects for run.

This specialization of runQuantum passes the output data ID to `run`.
"""
try:
inputs = butlerQC.get(inputRefs)
outputs = self.run(**inputs,
outputDataId=outputRefs.measurement.dataId)
if outputs.measurement is not None:
butlerQC.put(outputs, outputRefs)
else:
self.log.debug("Skipping measurement of %r on %s "
"as not applicable.", self, inputRefs)
except MetricComputationError:
self.log.error(
"Measurement of %r failed on %s->%s",
self, inputRefs, outputRefs, exc_info=True)
inputs = butlerQC.get(inputRefs)
outputs = self.run(**inputs,
outputDataId=outputRefs.measurement.dataId)
if outputs.measurement is not None:
butlerQC.put(outputs, outputRefs)
else:
self.log.debug("Skipping measurement of %r on %s "
"as not applicable.", self, inputRefs)
21 changes: 11 additions & 10 deletions python/lsst/verify/tasks/commonMetrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import astropy.units as u

import lsst.pex.config as pexConfig
from lsst.pipe.base import NoWorkFound

from lsst.verify import Measurement, Datum
from lsst.verify.tasks import MetricComputationError, MetadataMetricTask, \
Expand Down Expand Up @@ -125,13 +126,15 @@ def makeMeasurement(self, timings):

Returns
-------
measurement : `lsst.verify.Measurement` or `None`
measurement : `lsst.verify.Measurement`
The running time of the target method.

Raises
------
MetricComputationError
lsst.verify.tasks.MetricComputationError
Raised if the timing metadata are invalid.
lsst.pipe.base.NoWorkFound
Raised if no matching timing metadata found.
"""
if timings["StartTime"] is not None or timings["EndTime"] is not None:
try:
Expand All @@ -148,9 +151,7 @@ def makeMeasurement(self, timings):
meas.extras["end"] = Datum(timings["EndTimestamp"])
return meas
else:
self.log.info("Nothing to do: no timing information for %s found.",
self.config.target)
return None
raise NoWorkFound(f"Nothing to do: no timing information for {self.config.target} found.")


# Expose MemoryMetricConfig name because config-writers expect it
Expand Down Expand Up @@ -221,13 +222,15 @@ def makeMeasurement(self, memory):

Returns
-------
measurement : `lsst.verify.Measurement` or `None`
measurement : `lsst.verify.Measurement`
The maximum memory usage of the target method.

Raises
------
MetricComputationError
lsst.verify.tasks.MetricComputationError
Raised if the memory metadata are invalid.
lsst.pipe.base.NoWorkFound
Raised if no matching memory metadata found.
"""
if memory["EndMemory"] is not None:
try:
Expand All @@ -242,9 +245,7 @@ def makeMeasurement(self, memory):
meas.notes['estimator'] = 'utils.timer.timeMethod'
return meas
else:
self.log.info("Nothing to do: no memory information for %s found.",
self.config.target)
return None
raise NoWorkFound(f"Nothing to do: no memory information for {self.config.target} found.")

def _addUnits(self, memory, version):
"""Represent memory usage in correct units.
Expand Down
25 changes: 11 additions & 14 deletions python/lsst/verify/tasks/metadataMetricTask.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,6 @@ class AbstractMetadataMetricTask(MetricTask):
-----
This class should be customized by overriding `getInputMetadataKeys`
and `run`.

This class makes no assumptions about how to handle missing data;
`run` may be called with `None` values, and is responsible
for deciding how to deal with them.
"""
# Design note: getInputMetadataKeys and MetadataMetricTask.makeMeasurement
# are overrideable methods rather than subtask(s) to keep the configs for
Expand Down Expand Up @@ -180,7 +176,7 @@ def extractMetadata(metadata, metadataKeys):
Parameters
----------
metadata : `lsst.pipe.base.TaskMetadata`
A metadata object, assumed not `None`.
A metadata object.
metadataKeys : `dict` [`str`, `str`]
Keys are arbitrary labels, values are metadata keys (or their
substrings) in the format of
Expand Down Expand Up @@ -229,10 +225,6 @@ class MetadataMetricTask(AbstractMetadataMetricTask):
-----
This class should be customized by overriding `getInputMetadataKeys`
and `makeMeasurement`. You should not need to override `run`.

This class makes no assumptions about how to handle missing data;
`makeMeasurement` may be called with `None` values, and is responsible
for deciding how to deal with them.
"""
# Design note: getInputMetadataKeys and makeMeasurement are overrideable
# methods rather than subtask(s) to keep the configs for
Expand Down Expand Up @@ -264,14 +256,18 @@ def makeMeasurement(self, values):
lsst.verify.tasks.MetricComputationError
Raised if an algorithmic or system error prevents calculation of
the metric. See `run` for expected behavior.
lsst.pipe.base.NoWorkFound
Raised if the metric is ill-defined or otherwise inapplicable.
Typically this means that the pipeline step or option being
measured was not run.
"""

def run(self, metadata):
"""Compute a measurement from science task metadata.

Parameters
----------
metadata : `lsst.pipe.base.TaskMetadata` or `None`
metadata : `lsst.pipe.base.TaskMetadata`
A metadata object for the unit of science processing to use for
this metric, or a collection of such objects if this task combines
many units of processing into a single metric.
Expand All @@ -289,6 +285,10 @@ def run(self, metadata):
lsst.verify.tasks.MetricComputationError
Raised if the strings returned by `getInputMetadataKeys` match
more than one key in any metadata object.
lsst.pipe.base.NoWorkFound
Raised if the metric is ill-defined or otherwise inapplicable.
Typically this means that the pipeline step or option being
measured was not run.

Notes
-----
Expand All @@ -299,9 +299,6 @@ def run(self, metadata):
"""
metadataKeys = self.getInputMetadataKeys(self.config)

if metadata is not None:
data = self.extractMetadata(metadata, metadataKeys)
else:
data = {dataName: None for dataName in metadataKeys}
data = self.extractMetadata(metadata, metadataKeys)

return Struct(measurement=self.makeMeasurement(data))
41 changes: 17 additions & 24 deletions python/lsst/verify/tasks/metricTask.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ def run(self, **kwargs):
- ``measurement``: the value of the metric
(`lsst.verify.Measurement` or `None`). This method is not
responsible for adding mandatory metadata (e.g., the data ID);
this is handled by the caller.
this is handled by the caller. `None` may be used to indicate
that a metric is undefined or irrelevant instead of raising
`~lsst.pipe.base.NoWorkFound`.

Raises
------
Expand All @@ -154,34 +156,25 @@ def run(self, **kwargs):
more specific exception describing the root cause.

Not having enough data for a metric to be applicable is not an
error, and should not trigger this exception.

Notes
-----
All input data must be treated as optional. This maximizes the
``MetricTask``'s usefulness for incomplete pipeline runs or runs with
optional processing steps. If a metric cannot be calculated because
the necessary inputs are missing, the ``MetricTask`` must return `None`
in place of the measurement.
error, and should raise ``NoWorkFound`` (see below) instead of
this exception.
lsst.pipe.base.NoWorkFound
Raised if the metric is ill-defined or otherwise inapplicable to
the data. Typically this means that the pipeline step or option
being measured was not run.
"""

def runQuantum(self, butlerQC, inputRefs, outputRefs):
"""Do Butler I/O to provide in-memory objects for run.

This specialization of runQuantum performs error-handling specific to
MetricTasks. Most or all of this functionality may be moved to
activators in the future.
MetricTasks.
"""
# Synchronize changes to this method with ApdbMetricTask
try:
inputs = butlerQC.get(inputRefs)
outputs = self.run(**inputs)
if outputs.measurement is not None:
butlerQC.put(outputs, outputRefs)
else:
self.log.debug("Skipping measurement of %r on %s "
"as not applicable.", self, inputRefs)
except MetricComputationError:
self.log.error(
"Measurement of %r failed on %s->%s",
self, inputRefs, outputRefs, exc_info=True)
inputs = butlerQC.get(inputRefs)
outputs = self.run(**inputs)
if outputs.measurement is not None:
butlerQC.put(outputs, outputRefs)
else:
self.log.debug("Skipping measurement of %r on %s "
"as not applicable.", self, inputRefs)