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

Output jointcal metrics via a metrics logger

    Details

    • Story Points:
      5
    • Sprint:
      Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6, Alert Production F17 - 9, Alert Production F17 - 10, Alert Production F17 - 11
    • Team:
      Alert Production

      Description

      Until we have butler metrics persistence system, we can use a dedicated logger to output each product's metrics. Since jointcal is the testbed for the new metrics system, we'll use it as an example for how to produce those logs.

        Attachments

          Issue Links

            Activity

            Parejkoj John Parejko created issue -
            Parejkoj John Parejko made changes -
            Field Original Value New Value
            Epic Link DM-8477 [ 28109 ]
            Parejkoj John Parejko made changes -
            Epic Link DM-8477 [ 28109 ] DM-9539 [ 30459 ]
            Parejkoj John Parejko made changes -
            Team SQuaRE [ 10302 ] Alert Production [ 10300 ]
            Parejkoj John Parejko made changes -
            Sprint Alert Production S17 - 2 [ 361 ]
            Hide
            Parejkoj John Parejko added a comment -

            Started this: fortunately, I already have a dict of metrics for the current unittests that will serve as a handy starting point.

            Show
            Parejkoj John Parejko added a comment - Started this: fortunately, I already have a dict of metrics for the current unittests that will serve as a handy starting point.
            Parejkoj John Parejko made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Parejkoj John Parejko made changes -
            Story Points 4
            krughoff Simon Krughoff made changes -
            Sprint Alert Production S17 - 2 [ 361 ] Alert Production S17 - 2, Alert Production S17 - 3 [ 361, 605 ]
            krughoff Simon Krughoff made changes -
            Rank Ranked higher
            Hide
            Parejkoj John Parejko added a comment -

            Comment from Jonathan Sick: put the import lsst.validate.base in a try: except: block, and only call output_measurements if it is possible to do so. That way, we can merge this to master, while continuing to do the necessary work in the validate.base epic branch.

            Show
            Parejkoj John Parejko added a comment - Comment from Jonathan Sick : put the import lsst.validate.base in a try: except: block, and only call output_measurements if it is possible to do so. That way, we can merge this to master, while continuing to do the necessary work in the validate.base epic branch.
            Hide
            Parejkoj John Parejko added a comment -

            Please take a look. It's mostly changes to the names of the metrics to match validate_metrics (also reflected in the tests).

            If the output_measurements API changes, we'll have to modify the call here as well, but we can deal with that when the time comes.

            Show
            Parejkoj John Parejko added a comment - Please take a look. It's mostly changes to the names of the metrics to match validate_metrics (also reflected in the tests). If the output_measurements API changes, we'll have to modify the call here as well, but we can deal with that when the time comes.
            Parejkoj John Parejko made changes -
            Reviewers Jonathan Sick [ jsick ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            krughoff Simon Krughoff made changes -
            Sprint Alert Production S17 - 2, Alert Production S17 - 3 [ 361, 605 ] Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4 [ 361, 605, 610 ]
            krughoff Simon Krughoff made changes -
            Rank Ranked higher
            krughoff Simon Krughoff made changes -
            Sprint Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4 [ 361, 605, 610 ] Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5 [ 361, 605, 610, 613 ]
            krughoff Simon Krughoff made changes -
            Rank Ranked higher
            Hide
            Parejkoj John Parejko added a comment -

            Now that Jonathan Sick's mostly done with verify, the new method to call is lsst.verify.output_quantities. Hopefully that's all that I need to do right now (plus, maybe, add some metadata).

            Show
            Parejkoj John Parejko added a comment - Now that Jonathan Sick 's mostly done with verify , the new method to call is lsst.verify.output_quantities . Hopefully that's all that I need to do right now (plus, maybe, add some metadata).
            Parejkoj John Parejko made changes -
            Link This issue is blocked by RFC-345 [ RFC-345 ]
            krughoff Simon Krughoff made changes -
            Sprint Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5 [ 361, 605, 610, 613 ] Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6 [ 361, 605, 610, 613, 616 ]
            krughoff Simon Krughoff made changes -
            Rank Ranked higher
            krughoff Simon Krughoff made changes -
            Sprint Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6 [ 361, 605, 610, 613, 616 ] Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6, Alert Production F17 - 7 [ 361, 605, 610, 613, 616, 626 ]
            krughoff Simon Krughoff made changes -
            Rank Ranked higher
            Parejkoj John Parejko made changes -
            Sprint Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6, Alert Production F17 - 7 [ 361, 605, 610, 613, 616, 626 ] Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6 [ 361, 605, 610, 613, 616 ]
            Parejkoj John Parejko made changes -
            Rank Ranked lower
            Parejkoj John Parejko made changes -
            Sprint Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6 [ 361, 605, 610, 613, 616 ] Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6, Alert Production F17 - 9 [ 361, 605, 610, 613, 616, 639 ]
            Parejkoj John Parejko made changes -
            Rank Ranked lower
            Hide
            swinbank John Swinbank added a comment -

            John Parejko, Jonathan Sick — this has been in review for quite a while. Is it still relevant?

            Show
            swinbank John Swinbank added a comment - John Parejko , Jonathan Sick — this has been in review for quite a while. Is it still relevant?
            swinbank John Swinbank made changes -
            Sprint Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6, Alert Production F17 - 9 [ 361, 605, 610, 613, 616, 639 ] Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6, Alert Production F17 - 9, Alert Production F17 - 10 [ 361, 605, 610, 613, 616, 639, 643 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            Hide
            krughoff Simon Krughoff added a comment -

            John Swinbank I think I'm picking this up.

            Show
            krughoff Simon Krughoff added a comment - John Swinbank I think I'm picking this up.
            Hide
            Parejkoj John Parejko added a comment -

            After some chatting with Krzysztof Findeisen, it looks like the missing work here is to import lsst.verify, drop lsst.validate, change self.metrics to self.measurements (for naming consistency) and call lsst.verify.output_quantities('jointcal', self.measurements) at the end of jointcal.run(). This will let us trial the verify system, and possibly plug in to SQuaSH once that connection is completed, and it shouldn't require too much effort to change if the Job system or Measurement persistence system changes in the future.

            Show
            Parejkoj John Parejko added a comment - After some chatting with Krzysztof Findeisen , it looks like the missing work here is to import lsst.verify , drop lsst.validate , change self.metrics to self.measurements (for naming consistency) and call lsst.verify.output_quantities('jointcal', self.measurements) at the end of jointcal.run() . This will let us trial the verify system, and possibly plug in to SQuaSH once that connection is completed, and it shouldn't require too much effort to change if the Job system or Measurement persistence system changes in the future.
            Hide
            krughoff Simon Krughoff added a comment -

            John Parejko are you looking for me to take on these changes? I'm happy to do it, and have actually started looking into it. However, I don't want to step on toes.

            Show
            krughoff Simon Krughoff added a comment - John Parejko are you looking for me to take on these changes? I'm happy to do it, and have actually started looking into it. However, I don't want to step on toes.
            Hide
            Parejkoj John Parejko added a comment -

            Either way: you said above that you were picking it up, so feel free. I'd be happy to review the finished result, if you like.

            Show
            Parejkoj John Parejko added a comment - Either way: you said above that you were picking it up, so feel free. I'd be happy to review the finished result, if you like.
            Hide
            krughoff Simon Krughoff added a comment -

            Sounds good. Let's handle it like that.

            Show
            krughoff Simon Krughoff added a comment - Sounds good. Let's handle it like that.
            krughoff Simon Krughoff made changes -
            Link This issue relates to DM-12253 [ DM-12253 ]
            Hide
            krughoff Simon Krughoff added a comment -

            John Parejko I have rebased to master (though master has moved on), would you be willing to take a look at this. The changes are not huge, but allow for at least some metrics to make it out of jointcal.
            My main question is if this is far enough along to call this done and get it running in our validate_drp runs.

            Show
            krughoff Simon Krughoff added a comment - John Parejko I have rebased to master (though master has moved on), would you be willing to take a look at this. The changes are not huge, but allow for at least some metrics to make it out of jointcal . My main question is if this is far enough along to call this done and get it running in our validate_drp runs.
            Hide
            Parejkoj John Parejko added a comment -

            You've still got a user branch: should that be deleted? The version in the ticket branch is still my old code (using lsst.validate.base).

            I don't think these changes have anything to do with running in validate_drp, however. They are what we want for tracking in squash.

            Show
            Parejkoj John Parejko added a comment - You've still got a user branch: should that be deleted? The version in the ticket branch is still my old code (using lsst.validate.base ). I don't think these changes have anything to do with running in validate_drp , however. They are what we want for tracking in squash.
            Hide
            krughoff Simon Krughoff added a comment -

            Sorry, the branch you should look at are in my user branch. If you wish, I can rename to the ticket branch. I mention `validate_drp` because I want to put a `jointcal` run in the `validate_drp` script so we get squash outputs whenever that is run.

            Show
            krughoff Simon Krughoff added a comment - Sorry, the branch you should look at are in my user branch. If you wish, I can rename to the ticket branch. I mention `validate_drp` because I want to put a `jointcal` run in the `validate_drp` script so we get squash outputs whenever that is run.
            Hide
            krughoff Simon Krughoff added a comment - - edited

            Branch is renamed. I'll create the PR. Actually, just noticed you created it already. Does it figure that out on a branch rename?

            Show
            krughoff Simon Krughoff added a comment - - edited Branch is renamed. I'll create the PR. Actually, just noticed you created it already. Does it figure that out on a branch rename?
            krughoff Simon Krughoff made changes -
            Assignee John Parejko [ parejkoj ] Simon Krughoff [ krughoff ]
            Hide
            krughoff Simon Krughoff added a comment -

            Moving this back to in progress as master has moved on too far to test. Need to rebase.

            Show
            krughoff Simon Krughoff added a comment - Moving this back to in progress as master has moved on too far to test. Need to rebase.
            krughoff Simon Krughoff made changes -
            Status In Review [ 10004 ] In Progress [ 3 ]
            swinbank John Swinbank made changes -
            Sprint Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6, Alert Production F17 - 9, Alert Production F17 - 10 [ 361, 605, 610, 613, 616, 639, 643 ] Alert Production S17 - 2, Alert Production S17 - 3, Alert Production S17 - 4, Alert Production S17 - 5, Alert Production S17 - 6, Alert Production F17 - 9, Alert Production F17 - 10, Alert Production F17 - 11 [ 361, 605, 610, 613, 616, 639, 643, 644 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            Hide
            Parejkoj John Parejko added a comment -

            So, master has moved on even further, in a way that's going to make it even harder for you to get the rebase to be clean. If you can restore the original branch, I think I can do a manual rebase relatively easily myself, and then hand it back to you to finish tidying up.

            Show
            Parejkoj John Parejko added a comment - So, master has moved on even further, in a way that's going to make it even harder for you to get the rebase to be clean. If you can restore the original branch, I think I can do a manual rebase relatively easily myself, and then hand it back to you to finish tidying up.
            Hide
            krughoff Simon Krughoff added a comment -

            I have restored the branch, I think. Let me know if you need something else.

            Show
            krughoff Simon Krughoff added a comment - I have restored the branch, I think. Let me know if you need something else.
            Hide
            Parejkoj John Parejko added a comment - - edited

            Simon Krughoff I've redone the name changes (effectively rebase to master, but really just redo work on master). On slack you said you could convert this to use verify.Job, so I'll leave that to you.

            Let me know if you want help dealing with the existing unittests of measurements.

            Show
            Parejkoj John Parejko added a comment - - edited Simon Krughoff I've redone the name changes (effectively rebase to master, but really just redo work on master). On slack you said you could convert this to use verify.Job , so I'll leave that to you. Let me know if you want help dealing with the existing unittests of measurements.
            Hide
            krughoff Simon Krughoff added a comment -

            As mentioned on the PR for jointcal, I believe I'm done. Reassigning to John Parejko for final signoff.

            Show
            krughoff Simon Krughoff added a comment - As mentioned on the PR for jointcal , I believe I'm done. Reassigning to John Parejko for final signoff.
            krughoff Simon Krughoff made changes -
            Reviewers Jonathan Sick [ jsick ] John Parejko [ parejkoj ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            krughoff Simon Krughoff added a comment -

            I have merged the verify version of this since it was pretty much orthogonal.

            Show
            krughoff Simon Krughoff added a comment - I have merged the verify version of this since it was pretty much orthogonal.
            Hide
            Parejkoj John Parejko added a comment -

            I've rebased jointcal to master, squashed all the changes, and cleaned it up a bit. I won't push any new jointcal changes until this gets merged, to avoid further conflicts. I also fixed Simon Krughoff's truncated commit message in obs_base, rebased it to master, made the PR for it.

            New Jenkins run here: https://ci.lsst.codes/job/stack-os-matrix/27092/

            Simon Krughoff can you please give the PRs a quick once-over now that I've rebased them, to make sure I didn't fowl anything up? Once you give the word and if the above jenkins run passes, either of us should be ok to merge the remaining 3 PRs.

            Show
            Parejkoj John Parejko added a comment - I've rebased jointcal to master, squashed all the changes, and cleaned it up a bit. I won't push any new jointcal changes until this gets merged, to avoid further conflicts. I also fixed Simon Krughoff 's truncated commit message in obs_base, rebased it to master, made the PR for it. New Jenkins run here: https://ci.lsst.codes/job/stack-os-matrix/27092/ Simon Krughoff can you please give the PRs a quick once-over now that I've rebased them, to make sure I didn't fowl anything up? Once you give the word and if the above jenkins run passes, either of us should be ok to merge the remaining 3 PRs.
            Hide
            Parejkoj John Parejko added a comment -

            Happier jenkins: https://ci.lsst.codes/job/stack-os-matrix/27095/

            Simon Krughoff cleaned up the one comment on obs_base.

            Merged and done!

            Show
            Parejkoj John Parejko added a comment - Happier jenkins: https://ci.lsst.codes/job/stack-os-matrix/27095/ Simon Krughoff cleaned up the one comment on obs_base. Merged and done!
            Parejkoj John Parejko made changes -
            Resolution Done [ 10000 ]
            Status In Review [ 10004 ] Done [ 10002 ]
            krughoff Simon Krughoff made changes -
            Story Points 4 5

              People

              • Assignee:
                krughoff Simon Krughoff
                Reporter:
                Parejkoj John Parejko
                Reviewers:
                John Parejko
                Watchers:
                Angelo Fausti, Frossie Economou, John Parejko, John Swinbank, Jonathan Sick, Michael Wood-Vasey, Simon Krughoff, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                8 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel