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

enable jointcal config writing

    XMLWordPrintable

    Details

      Description

      There is a block of code in jointcal that disables the persisting of the config and metadata. We should remove that code and add a dataset description for jointcal configs.

      This is the block in question:

      # We don't need to persist config and metadata at this stage.
      # In this way, we don't need to put a specific entry in the camera mapper policy file
      def _getConfigName(self):
      return None
       
      def _getMetadataName(self):
      return None
      

       

        Attachments

          Activity

          Hide
          erykoff Eli Rykoff added a comment -

          I think that we want to resurrect this issue now that we're looking to put jointcal into production mode. The config saving shouldn't be too difficult, but I don't know if needs a tract/filter template.

          Show
          erykoff Eli Rykoff added a comment - I think that we want to resurrect this issue now that we're looking to put jointcal into production mode. The config saving shouldn't be too difficult, but I don't know if needs a tract/filter template.
          Hide
          Parejkoj John Parejko added a comment - - edited

          Eli Rykoff: Ask, and ye shall receive (and be marked reviewer)! I've only implemented the config persistence here, as I don't know if we need metadata persistence yet, and the metadata part would need a more complicated template. I think the main thing we'd want the template for is tracking runtime metrics, which would be good, but we're not actually trying to do it yet.

          Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29517/pipeline

          Show
          Parejkoj John Parejko added a comment - - edited Eli Rykoff : Ask, and ye shall receive (and be marked reviewer)! I've only implemented the config persistence here, as I don't know if we need metadata persistence yet, and the metadata part would need a more complicated template. I think the main thing we'd want the template for is tracking runtime metrics, which would be good, but we're not actually trying to do it yet. Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29517/pipeline
          Hide
          erykoff Eli Rykoff added a comment -

          Of course the `obs_base` PR didn't show up on Jira. Sigh.

          I'm glad I thought to look for it because I believe it has the wrong python class. And if the tests pass in spite of that, then there might need to be some additional check in the tests that the persisted datatype is correct, perhaps by reading it in and not just checking its existence. Eg:
          jointcalConfig = butler.get('jointcal_config') and checking the class of the returned value.

          Show
          erykoff Eli Rykoff added a comment - Of course the `obs_base` PR didn't show up on Jira. Sigh. I'm glad I thought to look for it because I believe it has the wrong python class. And if the tests pass in spite of that, then there might need to be some additional check in the tests that the persisted datatype is correct, perhaps by reading it in and not just checking its existence. Eg: jointcalConfig = butler.get('jointcal_config') and checking the class of the returned value.
          Hide
          Parejkoj John Parejko added a comment -

          Jira's picking up the obs_base PR now.

          Good catch on that. I just copied and pasted another Config and didn't even think to look at that line. Turns out, the Butler will raise if you try to read it, because the config file itself has an assert on the type of the config. I added some other tests to check that it's actually written with the test values. Please take another look.

          Show
          Parejkoj John Parejko added a comment - Jira's picking up the obs_base PR now. Good catch on that. I just copied and pasted another Config and didn't even think to look at that line. Turns out, the Butler will raise if you try to read it, because the config file itself has an assert on the type of the config. I added some other tests to check that it's actually written with the test values. Please take another look.
          Hide
          Parejkoj John Parejko added a comment -

          Thanks for the review. If we need metadata persistence, we can file a new ticket.

           Merged and done.

          Show
          Parejkoj John Parejko added a comment - Thanks for the review. If we need metadata persistence, we can file a new ticket.  Merged and done.

            People

            Assignee:
            Parejkoj John Parejko
            Reporter:
            Parejkoj John Parejko
            Reviewers:
            Eli Rykoff
            Watchers:
            Colin Slater, Eli Rykoff, John Parejko, John Swinbank, Simon Krughoff
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: