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

Try to get calibration source selection consistent between gen2 and gen3 middleware

    XMLWordPrintable

Details

    • 8
    • DRP S21a (Dec Jan)
    • Data Release Production
    • No

    Description

      A detailed comparison between gen2 and gen3 middleware data products up to and including Single Frame Processing was made on DM-28858. It was revealed there that there are small differences in the sources that actually get used in the photometric and astrometric calibrations of order a-few-to-a-dozen per CCD. This leads to small differences in the WCS solutions resulting in slightly different CCD pixel scales (up to ~0.08 mas), coordinate offsets (up to ~3.4 mas), and small differences in some photoCalibs (differences up to: calibrationMean: 2e-03 calibrationErr: 6e-07 instFluxAtZeroMagnitude: 1e+09). Since we are so close to bitwise parity up to this point, it would be really nice to get all the way there (especially considering the desire to feed exactly the same inputs to both middleware versions of the fgcm and jointcal external calibrations).  An attempt to match up the selection of sources used in the SFP photometric and astrometric solutions between the two middleware versions will be attempted here.

      Attachments

        Issue Links

          Activity

            No builds found.
            lauren Lauren MacArthur created issue -
            lauren Lauren MacArthur made changes -
            Field Original Value New Value
            Link This issue relates to DM-28858 [ DM-28858 ]
            lauren Lauren MacArthur made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            lauren Lauren MacArthur made changes -
            Link This issue relates to DM-28985 [ DM-28985 ]
            lauren Lauren MacArthur made changes -
            Link This issue is blocked by DM-28985 [ DM-28985 ]
            lauren Lauren MacArthur made changes -
            Remote Link This issue links to "Page (Confluence)" [ 27824 ]
            lauren Lauren MacArthur made changes -
            lauren Lauren MacArthur made changes -
            lauren Lauren MacArthur made changes -
            lauren Lauren MacArthur added a comment - - edited

            Allrighty...I think I've got this.  It was a bit difficult to track down.  I'll keep the dirty details to a minimum, but the adventure went something like this...

            As a starting point, I wanted to see what references were actually ending up in the loaded + trimmed astrometry reference catalogs, so I went in and manually output the tables returned by the loader. The following plots a comparison of loaded refCats between the Gen2 and Gen3 processing runs for two of the CCDs in the one visit I have tested so far (1228, HSC-I) [spoiler alert, it also includes the result of the fix on this branch as the green +'s!]:


            You can see that the Gen3 (orange x's) loader is clearly not padding the initial bbox. The first "bug" I noted in this regard was that the gen3 reference object loader was using a named parameter, bboxPadding, in the loadPixelBox function (the method called in the astrometric calibration's run method here). This should end up being the value set in the loader's config parameter pixelMargin (as defined here). However, this config is not accessed at all in the gen3 loader path and was just getting the default value of 100 (as opposed to 300)...so, bingo, just get that parameter to match the pixelMargin and all is good, yes? Well, no...playing around with forcing many values for bboxPadding produced the very same catalog (trimmed to the original, non-padded image wcs). I tracked this down to the filtering algorithm that was indeed (erroneously) using the original non-padded bbox to filter on. I've now gotten rid of bboxPadding altogether and have all reference loading paths respecting the padding value set in the pixelMargin config. So, with this fix, where do we stand? Drumroll, please...with the caveat that testing has only been done thus far on a single visit...for every comparison I've made (including image/variance/mask planes, PSFs, WCSs, photoCalibs, for which I also attached the identical refCat selection for photometric calibration for the same two CCDs, EVERY COLUMN IN THE src CATALOGS), the ONLY remaining difference in the src catalogs is the integer offset for the deblend_peakId entries noted in DM-28858. Everything else is now bitwise-identical up to the end of Single Frame Processing.

            lauren Lauren MacArthur added a comment - - edited Allrighty...I think I've got this.  It was a bit difficult to track down.  I'll keep the dirty details to a minimum, but the adventure went something like this... As a starting point, I wanted to see what references were actually ending up in the loaded + trimmed astrometry reference catalogs, so I went in and manually output the tables returned by the loader. The following plots a comparison of loaded refCats between the Gen2 and Gen3 processing runs for two of the CCDs in the one visit I have tested so far (1228, HSC-I) [spoiler alert, it also includes the result of the fix on this branch as the green +'s!] : You can see that the Gen3 (orange x's) loader is clearly not padding the initial bbox. The first "bug" I noted in this regard was that the gen3 reference object loader was using a named parameter, bboxPadding , in the loadPixelBox function (the method called in the astrometric calibration's run method here ). This should end up being the value set in the loader's config parameter pixelMargin (as defined here ). However, this config is not accessed at all in the gen3 loader path and was just getting the default value of 100 (as opposed to 300)...so, bingo, just get that parameter to match the pixelMargin and all is good, yes? Well, no...playing around with forcing many values for bboxPadding produced the very same catalog (trimmed to the original, non-padded image wcs). I tracked this down to the filtering algorithm that was indeed (erroneously) using the original non-padded bbox to filter on. I've now gotten rid of bboxPadding altogether and have all reference loading paths respecting the padding value set in the pixelMargin config. So, with this fix, where do we stand? Drumroll, please...with the caveat that testing has only been done thus far on a single visit...for every comparison I've made (including image/variance/mask planes, PSFs, WCSs, photoCalibs, for which I also attached the identical refCat selection for photometric calibration for the same two CCDs, EVERY COLUMN IN THE src CATALOGS), the ONLY remaining difference in the src catalogs is the integer offset for the deblend_peakId entries noted in DM-28858 . Everything else is now bitwise-identical up to the end of Single Frame Processing.
            lauren Lauren MacArthur added a comment - - edited

            Would you mind giving this a look?  I believe you were the main author of the original code, so you are best placed to know if my changes are ok.  Jenkins + ci_hsc is happy. I am going to continue to test this on a few more visits, but I think the code is ready for review.

            Two PRs:
            https://github.com/lsst/meas_algorithms/pull/231
            https://github.com/lsst/pipe_tasks/pull/480

            lauren Lauren MacArthur added a comment - - edited Would you mind giving this a look?  I believe you were the main author of the original code, so you are best placed to know if my changes are ok.  Jenkins + ci_hsc is happy . I am going to continue to test this on a few more visits, but I think the code is ready for review. Two PRs: https://github.com/lsst/meas_algorithms/pull/231 https://github.com/lsst/pipe_tasks/pull/480
            lauren Lauren MacArthur made changes -
            Reviewers Nate Lust [ nlust ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            lauren Lauren MacArthur made changes -
            Component/s meas_algorithms [ 10732 ]
            lauren Lauren MacArthur made changes -
            Labels gen3-middleware drp-parity gen3-middleware
            yusra Yusra AlSayyad made changes -
            Epic Link DM-27975 [ 442760 ] DM-29153 [ 458510 ]
            sullivan Ian Sullivan made changes -
            Link This issue relates to DM-29344 [ DM-29344 ]
            lauren Lauren MacArthur made changes -
            Remote Link This issue links to "Page (Confluence)" [ 28018 ]
            lauren Lauren MacArthur added a comment - - edited

            Ok, modulo some doc string specifics, I think I've got this updated according to the recommendations on the PR.  I did test runs on the two CCDs above and get the exact same filtered catalog, so I think all is good in terms of effective matched gen2/gen3 behavior.  Could you (i.e. nlust) give the meas_algorightms branch another look?

            New Jenkins + ci_hsc is running.

            lauren Lauren MacArthur added a comment - - edited Ok, modulo some doc string specifics, I think I've got this updated according to the recommendations on the PR.  I did test runs on the two CCDs above and get the exact same filtered catalog, so I think all is good in terms of effective matched gen2/gen3 behavior.  Could you (i.e. nlust ) give the meas_algorightms branch another look? New Jenkins + ci_hsc is running.
            nlust Nate Lust added a comment -

            Thanks for getting that all turned around so fast, looks good to go to me

            nlust Nate Lust added a comment - Thanks for getting that all turned around so fast, looks good to go to me
            nlust Nate Lust made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]

            Great, thanks Nate! I just spotted one Box2I -> Box2D change for consistency, so I've pushed that and kicked off another Jenkins (abundance of caution...). I'll merge once that passes.

            lauren Lauren MacArthur added a comment - Great, thanks Nate! I just spotted one Box2I -> Box2D change for consistency, so I've pushed that and kicked off another Jenkins (abundance of caution...). I'll merge once that passes.
            nlust Nate Lust added a comment -

            Sounds good!

            nlust Nate Lust added a comment - Sounds good!

            Final Jenkins passed...merged to master and done!

            lauren Lauren MacArthur added a comment - Final Jenkins passed...merged to master and done!
            lauren Lauren MacArthur made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            lauren Lauren MacArthur made changes -
            Story Points 8
            lauren Lauren MacArthur made changes -
            Link This issue relates to DM-29819 [ DM-29819 ]
            lauren Lauren MacArthur made changes -
            Remote Link This issue links to "Page (Confluence)" [ 28348 ]
            lauren Lauren MacArthur made changes -
            Link This issue relates to DM-30030 [ DM-30030 ]

            People

              lauren Lauren MacArthur
              lauren Lauren MacArthur
              Nate Lust
              Jim Bosch, Lauren MacArthur, Nate Lust
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.