# Output jointcal metrics via a metrics logger

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
5
• Sprint:
• Team:

## 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.

## Activity

Hide
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
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.
Hide
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
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
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
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.
Hide
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
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).
Hide
John Swinbank added a comment -

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

Show
John Swinbank added a comment - John Parejko , Jonathan Sick — this has been in review for quite a while. Is it still relevant?
Hide
Simon Krughoff added a comment -

John Swinbank I think I'm picking this up.

Show
Simon Krughoff added a comment - John Swinbank I think I'm picking this up.
Hide
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
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
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
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
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
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
Simon Krughoff added a comment -

Sounds good. Let's handle it like that.

Show
Simon Krughoff added a comment - Sounds good. Let's handle it like that.
Hide
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
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
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
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
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
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
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
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?
Hide
Simon Krughoff added a comment -

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

Show
Simon Krughoff added a comment - Moving this back to in progress as master has moved on too far to test. Need to rebase.
Hide
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
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
Simon Krughoff added a comment -

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

Show
Simon Krughoff added a comment - I have restored the branch, I think. Let me know if you need something else.
Hide
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
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
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
Simon Krughoff added a comment - As mentioned on the PR for jointcal , I believe I'm done. Reassigning to John Parejko for final signoff.
Hide
Simon Krughoff added a comment -

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

Show
Simon Krughoff added a comment - I have merged the verify version of this since it was pretty much orthogonal.
Hide
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
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
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
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!

## People

• Assignee:
Simon Krughoff
Reporter:
John Parejko
Reviewers:
John Parejko
Watchers:
Angelo Fausti, Frossie Economou, John Parejko, John Swinbank, Jonathan Sick, Michael Wood-Vasey, Simon Krughoff, Tim Jenness