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

CalibrateTask should optionally not compute initial astrometry

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Invalid
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pipe_tasks
    • Labels:
      None

      Description

      For processing data which already has a good WCS (such as CFHT and SDSS) it is unnecessary to perform an initial fit of astrometry before computing the PSF. Furthermore it is risky to do so because the astrometry fitter may fail.

      At present CFHT handles this by overriding the CalibrateTask with its own variant task and a variant run method. However, the amount of code duplication required is unacceptable.

      To avoid this I suggest we add the ability to disable the initial astrometric fit.

      I also think we should Refactor CalibrateTask so its run method is much simpler and easier to override. But even if we do that (and perhaps it should be a separate ticket) it is such a common need to not run the initial astrometry that I think one should not have to override the run method to do it.

        Attachments

          Issue Links

            Activity

            No builds found.
            rowen Russell Owen created issue -
            rowen Russell Owen made changes -
            Field Original Value New Value
            Description For processing data which already has a good WCS (such as CFHT and SDSS) it is unnecessary to perform an initial fit of astrometry before computing the PSF. Furthermore it is risky to do so because the astrometry fitter may fail.

            At present CFHT handles this by overriding the CalibrateTask with its own variant task and a variant run method. However, the amount of code duplication required is unacceptable.

            To avoid this I suggest we add the ability to disable the initial astrometric fit.

            I also think we should Refactor CalibrateTask so its run method is much simpler and easier to override. But even if we do that (and perhaps it should be a separate ticket) it is such a common need to not run the initial astrometry that I think one should not have to override the run method to do it.

            Some thoughts on how to disable the initial astrometric fit:
            - One obvious choice is to add a calibration parameter to control this. It would then be overridden in the appropriate obs_* package. This provides flexibility, which might be useful for testing, but risks that the parameter will be mis-set (e.g. if CalibrateTask is used by more than one task then additional overrides become necessary).
            - One could imagine more hard-coded designs where the setting is fixed for a given obs_ package, e.g. by adding a class variable flag or constructor parameter.
            For processing data which already has a good WCS (such as CFHT and SDSS) it is unnecessary to perform an initial fit of astrometry before computing the PSF. Furthermore it is risky to do so because the astrometry fitter may fail.

            At present CFHT handles this by overriding the CalibrateTask with its own variant task and a variant run method. However, the amount of code duplication required is unacceptable.

            To avoid this I suggest we add the ability to disable the initial astrometric fit.

            I also think we should Refactor CalibrateTask so its run method is much simpler and easier to override. But even if we do that (and perhaps it should be a separate ticket) it is such a common need to not run the initial astrometry that I think one should not have to override the run method to do it.
            Hide
            swinbank John Swinbank added a comment -

            We anticipate a significant redesign of the major pipeline tasks – which would include this – in the Winter 2016 cycle. See DLP-495 & DLP-496 for details.

            Show
            swinbank John Swinbank added a comment - We anticipate a significant redesign of the major pipeline tasks – which would include this – in the Winter 2016 cycle. See DLP-495 & DLP-496 for details.
            swinbank John Swinbank made changes -
            Link This issue relates to DLP-495 [ DLP-495 ]
            swinbank John Swinbank made changes -
            Team Data Release Production [ 10301 ]
            swinbank John Swinbank made changes -
            Epic Link DM-3579 [ 19676 ]
            swinbank John Swinbank made changes -
            Epic Link DM-3579 [ 19676 ] DM-3882 [ 20177 ]
            Hide
            krughoff Simon Krughoff added a comment -

            This came up as one of the requirements for the ProcessCcdTask. Rather than have that work in two places, we are putting the work for refactoring that task in one story, DM-4692.

            Show
            krughoff Simon Krughoff added a comment - This came up as one of the requirements for the ProcessCcdTask. Rather than have that work in two places, we are putting the work for refactoring that task in one story, DM-4692 .
            krughoff Simon Krughoff made changes -
            Resolution Done [ 10000 ]
            Status To Do [ 10001 ] Invalid [ 11005 ]
            krughoff Simon Krughoff made changes -
            Link This issue duplicates DM-4692 [ DM-4692 ]

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              rowen Russell Owen
              Watchers:
              Dominique Boutigny, Jim Bosch, John Swinbank, Paul Price, Russell Owen, Simon Krughoff, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.