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

Make task config defaults sensible for all cameras

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      In the process of getting processCcd, jointcal, and multiband to work with obs_mosaic data, we found several cases where a default task configuration for some process did not work with our data. And we found in some cases that a default setting only worked with one camera (probably the one for which the config setting had been developed).

      Other cameras seem to work around these kinds of problems by having their config settings in obs_*/config. While this works, it would be better I think to have default setting which works with most cameras. It would certainly be more friendly to new pipeline developers.

      I will start by creating an RFC to create discussion on this issue, then let the result of the RFC decide what if any work needs to be done.

        Attachments

          Issue Links

            Activity

            Hide
            pgee Perry Gee added a comment -

            Meredith Rawls The other thing is that doApplyUberCal used to mean to apply meas_mosaic. But meas_mosaic doesn't currently work, except with HSC. The correct way to do this would have been to have a subtask to do the astrometry correction, which could be set to either meas_mosaic or jointcal. The use of a subclass of MakeTempCoaddExpTask for using jointcal was kind of a hack, and will eventually be removed.

            Show
            pgee Perry Gee added a comment - Meredith Rawls The other thing is that doApplyUberCal used to mean to apply meas_mosaic. But meas_mosaic doesn't currently work, except with HSC. The correct way to do this would have been to have a subtask to do the astrometry correction, which could be set to either meas_mosaic or jointcal. The use of a subclass of MakeTempCoaddExpTask for using jointcal was kind of a hack, and will eventually be removed.
            Hide
            mrawls Meredith Rawls added a comment -

            Thanks for the clarifications! The doApplyUberCal makes sense, though I'm still not thrilled with the name. I left a formatting comment on the PR.

            I am fine with the maxFootprintSize changes too - I appreciate that the default behavior is safe in that it preserves the original behavior. Of course, in practice users will probably want to set it to some value larger than zero. To that point, I suggest adding a boilerplate comment in each obs_camera's config so that new users have an idea where the counterintuitive 0 default originates without having to dig through the commit history. Something like "This config overrides the likely-more-practical value of 2000 set in pipe_tasks in order to preserve old behavior."

            Show
            mrawls Meredith Rawls added a comment - Thanks for the clarifications! The doApplyUberCal makes sense, though I'm still not thrilled with the name. I left a formatting comment on the PR. I am fine with the maxFootprintSize changes too - I appreciate that the default behavior is safe in that it preserves the original behavior. Of course, in practice users will probably want to set it to some value larger than zero. To that point, I suggest adding a boilerplate comment in each obs_camera's config so that new users have an idea where the counterintuitive 0 default originates without having to dig through the commit history. Something like "This config overrides the likely-more-practical value of 2000 set in pipe_tasks in order to preserve old behavior."
            Hide
            pgee Perry Gee added a comment -

            I agree about the need for a comment. However, see Paul Price's comment above, which cause me to remove the comment from all of the config files. I did not feel prepared to fight it.

            Show
            pgee Perry Gee added a comment - I agree about the need for a comment. However, see Paul Price's comment above, which cause me to remove the comment from all of the config files. I did not feel prepared to fight it.
            Hide
            mrawls Meredith Rawls added a comment -

            Up to you, then, I suppose... I understand Paul's point but I disagree. That's why I suggested adding comments rather than insisting

            Show
            mrawls Meredith Rawls added a comment - Up to you, then, I suppose... I understand Paul's point but I disagree. That's why I suggested adding comments rather than insisting
            Hide
            pgee Perry Gee added a comment -

            I guess Paul is handling HSC, and he can have it the way he wants as far as I am concerned. obs_cfht already has this is their configs. But I can add the comment for obs_sdss and obs_decam so that if they encounter this, it won't create confusion.

            Show
            pgee Perry Gee added a comment - I guess Paul is handling HSC, and he can have it the way he wants as far as I am concerned. obs_cfht already has this is their configs. But I can add the comment for obs_sdss and obs_decam so that if they encounter this, it won't create confusion.

              People

              • Assignee:
                pgee Perry Gee
                Reporter:
                pgee Perry Gee
                Reviewers:
                Meredith Rawls
                Watchers:
                Jim Bosch, John Swinbank, Meredith Rawls, Paul Price, Perry Gee, Yusra AlSayyad
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel