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

Butler define-visits ignores collection

    XMLWordPrintable

    Details

    • Story Points:
      1
    • Team:
      Architecture
    • Urgent?:
      No

      Description

      It seems that the define-visits command line is not respecting the collections parameter and so every single call to define-visits runs on every single "exposure" for that instrument.

      This was noted on a community post and looking at the code the queryDataIds is not constrained by the collection at all. The collection is passed to the DefineVisitsTask but is only used to retrieve the relevant camera (which means it's ignored and the default camera is returned). If registry is not used to work out the WCS the collection is used again to get the WCS for the specific raw.

      This implies that there are really two collections of interest.

      • The collection constraining the queryDataIds and associated with the butler.get of raw.wcs (if needed).
      • The calibration collection used to locate the camera geometry.

      As currently implemented DefineVisitsTask is effectively ignoring the collections parameter because loadCamera falls back to the generic one and we use registry to calculate the WCS.

      The quick fix here is to add collections to the queryDataIds method in the script layer.

      Longer term we will need to consider adding a calibration collection parameter.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            Jim Bosch Asking you to review for when you get back. I don't think it's a time critical ticket (although we have one user on community who needs it). It's only really a one line change. My main reason for asking you is that I'm a bit concerned about burning "raw" into the command as the dataset type that is associated with exposures that become visits. There is code in defineVisits that also assumes "raw" but we never use it (and I think we should remove the code for that completely because asking for raw.wcs is never going to be efficient option).

            Show
            tjenness Tim Jenness added a comment - Jim Bosch Asking you to review for when you get back. I don't think it's a time critical ticket (although we have one user on community who needs it). It's only really a one line change. My main reason for asking you is that I'm a bit concerned about burning "raw" into the command as the dataset type that is associated with exposures that become visits. There is code in defineVisits that also assumes "raw" but we never use it (and I think we should remove the code for that completely because asking for raw.wcs is never going to be efficient option).
            Hide
            tjenness Tim Jenness added a comment -

            Nate Pease [X] would you mind taking over the review? Jim answered the question about finding the right dataset type on slack – there is no easy way to do it at the moment and most approaches are a bit of a hack (I could query the datasets in the collection one at a time until I find one that has "exposure" in the dimension but that would be a guess at best) and the right fix would probably be to wait for the per-collection dataset type implementation.

            Show
            tjenness Tim Jenness added a comment - Nate Pease [X] would you mind taking over the review? Jim answered the question about finding the right dataset type on slack – there is no easy way to do it at the moment and most approaches are a bit of a hack (I could query the datasets in the collection one at a time until I find one that has "exposure" in the dimension but that would be a guess at best) and the right fix would probably be to wait for the per-collection dataset type implementation.
            Hide
            npease Nate Pease [X] (Inactive) added a comment -

            Thanks for pinging me on Slack about this and sorry missed the email about it. Looks good.

            Show
            npease Nate Pease [X] (Inactive) added a comment - Thanks for pinging me on Slack about this and sorry missed the email about it. Looks good.

              People

              Assignee:
              tjenness Tim Jenness
              Reporter:
              tjenness Tim Jenness
              Reviewers:
              Nate Pease [X] (Inactive)
              Watchers:
              Jim Bosch, Nate Pease [X] (Inactive), Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins Builds

                  No builds found.