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

Contract-checker is unaware of connection template substitution

    XMLWordPrintable

    Details

    • Type: Story
    • Status: To Do
    • Resolution: Unresolved
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      The contract checker doesn't expand out the connection templates before checking them. How to reproduce.

      Running this pipeline:

      instrument: lsst.obs.lsst.LsstCamImSim
      description: Run Transform
      tasks:
          imageDifference:
              class: lsst.pipe.tasks.imageDifference.ImageDifferenceTask
              config:
                  doSelectSources: False
                  coaddName: goodSeeing
                  getTemplate.coaddName: goodSeeing
                  getTemplate.warpType: direct
                  connections.coaddName: goodSeeing
                  # Setting template 'coaddName' is not sufficient to satisfy contract
                  connections.coaddExposures: goodSeeingCoadd
                  connections.subtractedExposure: goodSeeingDiff_differenceExp
          transformDiaSourceCat:
              class: lsst.ap.association.TransformDiaSourceCatalogTask
              config:
                  connections.coaddName: goodSeeing
                  #connections.diffIm: goodSeeingDiff_differenceExp
       
      contracts:
        - imageDifference.connections.subtractedExposure == transformDiaSourceCat.connections.diffIm
      

      with e.g. this command:

      pipetask run -b /repo/dc2 -p transform.yaml -i 2.2i/runs/test-med-1/w_2021_16/DM-29899 -o u/yusra/transform -d "instrument='LSSTCam-imSim' AND visit=416955 AND detector=9" --register-dataset-types
      

      will fail with:

       raise pipelineIR.ContractError(f"Contract(s) '{contract.contract}' were not "
      lsst.pipe.base.pipelineIR.ContractError: Contract(s) 'imageDifference.connections.subtractedExposure == transformDiaSourceCat.connections.diffIm' were not satisfied 

      unless you either uncomment out the last line on of the configs OR remove the contract line in the pipeline file.  This means that the template  coaddName is  expanding out "goodSeeing" in the connections just fine, even though the contract checker complains.  

        Attachments

          Issue Links

            Activity

            Hide
            nlust Nate Lust added a comment -

            I am going to need to think about this one a little bit, because it is actually working as intended. The configuration values are being set on the config object, which is later used to format a connections class. You are not actually looking at the connections class, just the info that goes into constructing one. In other words, someone might change the the connections name to a different format template, and want to validate the template before substitution (though of course that would be less frequent, and so we should probably angle toward the least surprise even if it makes for different execution paths)

            Show
            nlust Nate Lust added a comment - I am going to need to think about this one a little bit, because it is actually working as intended. The configuration values are being set on the config object, which is later used to format a connections class. You are not actually looking at the connections class, just the info that goes into constructing one. In other words, someone might change the the connections name to a different format template, and want to validate the template before substitution (though of course that would be less frequent, and so we should probably angle toward the least surprise even if it makes for different execution paths)
            Hide
            nlust Nate Lust added a comment - - edited

            a workaround (verbose) for now can be:

            - imageDifference.connections.ConnectionsClass(config=imageDifference).subtractedExposure.name==\
              transformDiaSourceCat.connections.ConnectionsClass(config=transformDiaSourceCat).diffIm.name
            

            I will try to think of the right way to handle this in the checker going forward

            Show
            nlust Nate Lust added a comment - - edited a workaround (verbose) for now can be: - imageDifference.connections.ConnectionsClass(config=imageDifference).subtractedExposure.name==\ transformDiaSourceCat.connections.ConnectionsClass(config=transformDiaSourceCat).diffIm.name I will try to think of the right way to handle this in the checker going forward
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            FWIW, in the AP configs we wrote the analogous contract as

            imageDifference.connections.coaddName == transformDiaSrcCat.connections.coaddName
            

            This approach won't catch problems if you override connections to not use the template, but is much more portable when you don't (not least because you don't need to enumerate every single connection). Is this a contract style that's discouraged in general?

            Show
            krzys Krzysztof Findeisen added a comment - - edited FWIW, in the AP configs we wrote the analogous contract as imageDifference.connections.coaddName = = transformDiaSrcCat.connections.coaddName This approach won't catch problems if you override connections to not use the template, but is much more portable when you don't (not least because you don't need to enumerate every single connection). Is this a contract style that's discouraged in general?
            Hide
            mrawls Meredith Rawls added a comment -

            Bumping this, as the continued requirement to manually set a bunch of things to, e.g., `goodSeeing` (when that's your desired coaddName flavor instead of `deep`) in a pipeline yaml wasted a couple hours of my time today.

            Show
            mrawls Meredith Rawls added a comment - Bumping this, as the continued requirement to manually set a bunch of things to, e.g., `goodSeeing` (when that's your desired coaddName flavor instead of `deep`) in a pipeline yaml wasted a couple hours of my time today.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Note that this problem does not apply to pipeline parameters – you can mix parameterized and non-parameterized fields in contracts.

            Show
            krzys Krzysztof Findeisen added a comment - Note that this problem does not apply to pipeline parameters – you can mix parameterized and non-parameterized fields in contracts.

              People

              Assignee:
              nlust Nate Lust
              Reporter:
              yusra Yusra AlSayyad
              Watchers:
              John Parejko, Kenneth Herner, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Yusra AlSayyad
              Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:

                  Jenkins

                  No builds found.