Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-20974

Remove aggregation support/requirement from MetricTask

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: verify
    • Labels:
      None
    • Story Points:
      6
    • Sprint:
      AP F19-3 (Skipped)
    • Team:
      Alert Production

      Description

      As originally proposed in DMTN-098, MetricTasks are responsible for aggregating metrics to any desired level of granularity. This capability has never been used and is unlikely to be needed in the future (SQuaSH's Chronograph UI already supports aggregation implicitly). On the other hand, it makes MetricTasks much harder to write (in particular, they can have much more complicated failure modes) and requires flexibility in inputs that the Gen 3 framework considers undesirable.

      Give each MetricTask a well-defined granularity. This will require breaking, and therefore simultaneous, changes to:

      • The MetricTask API documentation
      • The MetricControllerTask code (should still support MetricTasks that do a many-to-one transformation, though none exist at present)
      • The MetricControllerTask API documentation
      • discussion of inputs and philosophy in DMTN-098
      • MetadataMetricTask and PpdbMetricTask
      • all concrete MetricTasks

      No changes to ap_verify should be necessary.

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment -

            Adding Nate Lust as a watcher, as DM-20692 may need to be rebased onto this ticket (depending on when it gets merged).

            Show
            krzys Krzysztof Findeisen added a comment - Adding Nate Lust as a watcher, as DM-20692 may need to be rebased onto this ticket (depending on when it gets merged).
            Hide
            krzys Krzysztof Findeisen added a comment -

            Note: build #30379 failed due to Astropy IERS connectivity problems. While that means the changes here are probably good, I should rerun Jenkins before merging.

            Show
            krzys Krzysztof Findeisen added a comment - Note: build #30379 failed due to Astropy IERS connectivity problems. While that means the changes here are probably good, I should rerun Jenkins before merging.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Hi Tim Jenness, would you be willing to review this 600-line change? The most substantial changes are in lsst.verify, with the other packages essentially being direct replacements of 1-element lists with scalars.

            Show
            krzys Krzysztof Findeisen added a comment - Hi Tim Jenness , would you be willing to review this 600-line change? The most substantial changes are in lsst.verify , with the other packages essentially being direct replacements of 1-element lists with scalars.
            Hide
            tjenness Tim Jenness added a comment -

            I've had a look at the code and it seems fine to me. I've never looked at verify or related tasks before so it took me while to try to work out what was going on. Being explicit about scalars versus array metrics seems like a good thing.

            Show
            tjenness Tim Jenness added a comment - I've had a look at the code and it seems fine to me. I've never looked at verify or related tasks before so it took me while to try to work out what was going on. Being explicit about scalars versus array metrics seems like a good thing.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Thanks for the review! Build #30402 passed.

            Show
            krzys Krzysztof Findeisen added a comment - Thanks for the review! Build #30402 passed.

              People

              • Assignee:
                krzys Krzysztof Findeisen
                Reporter:
                krzys Krzysztof Findeisen
                Reviewers:
                Tim Jenness
                Watchers:
                Krzysztof Findeisen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel