DM-16536: Migrate all metrics from ap.verify.measurements #63
Conversation
|
It came up in the RFC-550 discussion. Where would be a good place to document a decision like that? |
|
Oh I see. I don't entirely understand how that circular dependency comes up. I'll have to take a closer look at how these tasks work. Until I understand the nature of the issue I can't really say if or where it should be documented. |
|
|
|
Sorry for being confused, but I thought the hope would be to migrate |
|
Ah. Yes, that would work from a dependency point of view, but I thought the RFC consensus was that code associated with specific metrics would not go into |
|
@jonathansick so should I take any action on |
|
By all means, this is good to go.
So I definitely feel that something like AM1MetricTask, or some other very domain-specific measurement task, ought to belong in the package most associated the metric. But something like timing (if I understand the task's intent well enough) seems so broadly applicable that it could belong in |
|
@jonathansick Fine. Any preferences on whether |
|
Given the issue I just ran into with documentation builds, I'll merge as-is and open another ticket for moving |
This commit decouples the MetricsControllerTask invocation from the old-style measurement code, making it possible to request measurements at any granularity.
Since this commit removes all use of the old-style metrics, the corresponding files are deleted as well.
AutoJob is no longer used by any code in ap_verify. Since SQuaSH upload was part of AutoJob, but there is no compelling case to reproduce the feature in the new metrics code, all SQuaSH-related code has been removed. The --silent command-line argument is deprecated.
kfindeisen
force-pushed
the
tickets/DM-16536
branch
from
0a659fa
to
5cf28e7
Compare
This PR replaces all functions previously in
lsst.ap.verify.measurementswithMetricTaskequivalents in other packages (except forTimingMetricTask, which cannot be reasonably placed in any other Stack package). The hardcoded references insidemeasurements/compute_metrics.pyare replaced withMetricTaskConfiginstances; two default configs for supporting CI are provided inconfig/.Correctly handling
lsst.ap.association.TotalUnassociatedDiaObjectsMetricTaskrequired a significant rewrite of the post-ApPipeTaskcontrol flow, which had been written on the assumption that all metrics are calculated after every image. The current solution (hardcoding the existence of twoMetricsControllerTasks, one for image- and one for dataset-level metrics) is clumsy both for the user and for code maintainers, but I believe it is better to defer generalizing the code to another, more focused ticket.