Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-617

Update HSC's focal plane camera geometry to use units of millimeters

    XMLWordPrintable

    Details

      Description

      The obs_subaru camera geometry currently uses an effective focal plane scaling of "unity" (i.e. plateScale = 1 arcsec/mm, pixelSize = 1 mm). All other LSST camera geometries are defined in actual physical units: millimeters on the focal plane. For the work being done in DM-19943 (and see RFC-616), we need to have the correct CCD plate scale in the camera geometry to properly reflect the pixel-to-focal plane conversion.

      In addition to this unit conversion being a good idea in its own right, the investigations in DM-19943 rendered us fairly confident that there is an inconsistency in the current HSC cameraGeom itself that was actually responsible for most of the problems. Updating the units thus has the added bonus of eliminating the inconsistency without us having to debug it further. Furthermore, HSC's non-conventional unit system has caused mild headaches for a few developers (e.g. Eli Rykoff's calibration work on fgcmcal), so the switch will benefit them as well.

      There are a few package affected and will require accommodation. These have already been exposed and tested on DM-20289 for singleFrame and skyCorrection and, in the context of ci_hsc, through coaddition and multiband with the cursory tests there (but without running jointcal, which is not yet included in any of our CI scripts). A more rigorous battle testing is being performed on DM-20548 (the ticket triggered by this RFC) which will include a full run of at least one tract from the RC2 dataset (i.e. running through singleFrameDriver, skyCorrection, jointcal, coaddDriver, and multibandDriver) to make sure the results are consistent with that of the latest weekly reprocessing run.

      One item of note: this change will effectively break meas_mosaic. While updating it to accommodate the unit update is not completely out of the question, it would take significant effort which does not seem worthwhile given the adoption of jointcal as our “uber calibration” algorithm. As such, this RFC is also effectively proposing the official retirement of meas_mosaic.

        Attachments

          Issue Links

            Activity

            Hide
            erykoff Eli Rykoff added a comment -

            Show
            erykoff Eli Rykoff added a comment -
            Hide
            erykoff Eli Rykoff added a comment -

            This would fix DM-17862 and unblock DM-16490, so I'm 100% on board.  (The fgcmcal "minor headaches" in ticket form!) Thanks!

            Show
            erykoff Eli Rykoff added a comment - This would fix DM-17862 and unblock DM-16490 , so I'm 100% on board.  (The fgcmcal "minor headaches" in ticket form!) Thanks!
            Hide
            Parejkoj John Parejko added a comment -

            Big +1 from me.

            Show
            Parejkoj John Parejko added a comment - Big +1 from me.
            Hide
            rhl Robert Lupton added a comment -

            The yaml version of the hsc camera implements geometry in mm.  We discussed this with the Japanese, and while they are not happy I don't think that they are going to object too strongly.

            Show
            rhl Robert Lupton added a comment - The yaml version of the hsc camera implements geometry in mm.  We discussed this with the Japanese, and while they are not happy I don't think that they are going to object too strongly.
            Hide
            jmeyers314 Joshua Meyers added a comment -

            +1

            Show
            jmeyers314 Joshua Meyers added a comment - +1
            Hide
            womullan Wil O'Mullane added a comment -

            The board supports this and will set it to recommended when the deadline passes in two weeks.

            Show
            womullan Wil O'Mullane added a comment - The board supports this and will set it to recommended when the deadline passes in two weeks.
            Hide
            swinbank John Swinbank added a comment -

            Board recommended, as above.

            Show
            swinbank John Swinbank added a comment - Board recommended, as above.
            Hide
            gcomoretto Gabriele Comoretto [X] (Inactive) added a comment -

            This RFC shall be adopted since it is now triggering an issue.

            Show
            gcomoretto Gabriele Comoretto [X] (Inactive) added a comment - This RFC shall be adopted since it is now triggering an issue.
            Hide
            lauren Lauren MacArthur added a comment -

            The triggered ticket DM-20548 is now in review.  Are there any further formal approvals required on this RFC for me to be ok to merge that ticket once it has been approved by the reviewers?

            Show
            lauren Lauren MacArthur added a comment - The triggered ticket DM-20548 is now in review.  Are there any further formal approvals required on this RFC for me to be ok to merge that ticket once it has been approved by the reviewers?
            Hide
            gcomoretto Gabriele Comoretto [X] (Inactive) added a comment -

            No, the RFC status shall be set to "Adopted" when a triggering issue exists.

            Show
            gcomoretto Gabriele Comoretto [X] (Inactive) added a comment - No, the RFC status shall be set to "Adopted" when a triggering issue exists.
            Hide
            tjenness Tim Jenness added a comment -

            This RFC says that meas_mosaic will be retired but the RFC has been marked as implemented and meas_mosaic still seems to be in lsst_distrib.

            Show
            tjenness Tim Jenness added a comment - This RFC says that meas_mosaic will be retired but the RFC has been marked as implemented and meas_mosaic still seems to be in lsst_distrib.
            Hide
            erykoff Eli Rykoff added a comment -

            That is a very good point! I wonder how the implementation passed Jenkins? I guess this wasn't tested within meas_mosaic. In any event, I think that should happen!

            Show
            erykoff Eli Rykoff added a comment - That is a very good point! I wonder how the implementation passed Jenkins? I guess this wasn't tested within meas_mosaic . In any event, I think that should happen!
            Hide
            lauren Lauren MacArthur added a comment -

            I was just typing a note to ask about this (I was going to write a community post warning that, while it compiles, it would produce garbage, if anything, so should not be used).  I'm not sure of the official deprecation process for a repository.  Should this RFC be moved back to the "Approved" stage while I await instructions on how to do the official deprecation of meas_mosaic?

            Show
            lauren Lauren MacArthur added a comment - I was just typing a note to ask about this (I was going to write a community post warning that, while it compiles, it would produce garbage, if anything, so should not be used).  I'm not sure of the official deprecation process for a repository.  Should this RFC be moved back to the "Approved" stage while I await instructions on how to do the official deprecation of meas_mosaic ?
            Hide
            lauren Lauren MacArthur added a comment -

            meas_mosaic got a minimal update such that the tests pass, so Jenkins was indeed happy.

            Show
            lauren Lauren MacArthur added a comment - meas_mosaic got a minimal update such that the tests pass, so Jenkins was indeed happy.
            Hide
            lauren Lauren MacArthur added a comment -

            John Swinbank has graciously offered to take on the deprecation of meas_mosaic on DM-21053.

            Show
            lauren Lauren MacArthur added a comment - John Swinbank has graciously offered to take on the deprecation of meas_mosaic on DM-21053 .
            Hide
            swinbank John Swinbank added a comment -

            Apparently I don't have permission to set this RFC back to “adopted”. However, I agree with the premise that this RFC should not be regarded as complete until meas_mosaic has been dropped from lsst_distrib (not just deprecated, but removed). I'll take care of that on DM-21053.

            Show
            swinbank John Swinbank added a comment - Apparently I don't have permission to set this RFC back to “adopted”. However, I agree with the premise that this RFC should not be regarded as complete until meas_mosaic has been dropped from lsst_distrib (not just deprecated, but removed). I'll take care of that on DM-21053 .

              People

              Assignee:
              lauren Lauren MacArthur
              Reporter:
              lauren Lauren MacArthur
              Watchers:
              Chris Morrison, Eli Rykoff, Gabriele Comoretto [X] (Inactive), Jim Bosch, John Parejko, John Swinbank, Joshua Meyers, Lauren MacArthur, Paul Price, Robert Lupton, Tim Jenness, Wil O'Mullane, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              13 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.