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

CalibCombineConnections changes its quantum dimensions at construction

    XMLWordPrintable

    Details

    • Story Points:
      2
    • Epic Link:
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      cp_pipe's CalibCombineConnections modifies its quantum dimensions in its constructor, meaning that instances report different dimensions than the class does.

      And it seems that the code path of registering dataset types via a saved quantum graph (the metadata dataset type for this task in particular) looks at the class version.

      I'm inclined to say that the most immediate solution for this is to just have different CalibCombineTask and CalibCombineConnections subclasses for each set of dimensions. That's a bit of boilerplate, but it's also very easy to read and understand.

      We also probably ought to fix whatever code path is looking at the class dimensions anyway, and then also look for some way to prevent new code from trying to do that. I'll create a new ticket for the former, and add the latter to DM-29658, which seems closely related.

        Attachments

          Issue Links

            Activity

            Hide
            mrawls Meredith Rawls added a comment -

            Left comments on GitHub. The changes seem like a good simplification, and just two things to address:

            • Please move special per-camera pipelines to cp_pipe/pipelines subdirectories as per RFC-775
            • Please consider adding a useful error message for anyone who runs an old cpCombine pipeline that still defines "calibrationDimensions"
            Show
            mrawls Meredith Rawls added a comment - Left comments on GitHub. The changes seem like a good simplification, and just two things to address: Please move special per-camera pipelines to cp_pipe/pipelines subdirectories as per RFC-775 Please consider adding a useful error message for anyone who runs an old cpCombine pipeline that still defines "calibrationDimensions"
            Hide
            mrawls Meredith Rawls added a comment -

            Thanks for making DM-29981 for my first point. I understand the second point will cause more of a butler-y qgraph error, which we'll have to live with for anyone who asks for help using an old pipeline. Thanks!

            Show
            mrawls Meredith Rawls added a comment - Thanks for making DM-29981 for my first point. I understand the second point will cause more of a butler-y qgraph error, which we'll have to live with for anyone who asks for help using an old pipeline. Thanks!
            Hide
            czw Christopher Waters added a comment - - edited

            A minor change was necessary to fix the inputScales connection.  The _init_ block I deleted handled that previously, so I've simply added that bit back (keeping the dimension futzery deleted).  A new Jenkins should now pass: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34125/pipeline

            Quickly found another bug.
            https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34126/pipeline

            And another.

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

            Show
            czw Christopher Waters added a comment - - edited A minor change was necessary to fix the inputScales connection.  The _ init _ block I deleted handled that previously, so I've simply added that bit back (keeping the dimension futzery deleted).  A new Jenkins should now pass:  https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34125/pipeline Quickly found another bug. https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34126/pipeline And another. https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34136/pipeline/  
            Hide
            mrawls Meredith Rawls added a comment -

            I'm not familiar with inputScales, but thank you for taking care to make Jenkins happy. The small init block looks reasonable to me, fingers crossed for CI to pass this time around!

            Show
            mrawls Meredith Rawls added a comment - I'm not familiar with inputScales , but thank you for taking care to make Jenkins happy. The small init block looks reasonable to me, fingers crossed for CI to pass this time around!

              People

              Assignee:
              czw Christopher Waters
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Meredith Rawls
              Watchers:
              Christopher Waters, James Chiang, Jim Bosch, Meredith Rawls, Nate Lust
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: