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

    • Story Points:
      8
    • Epic Link:
    • Sprint:
      DRP S21a (Dec Jan)
    • Team:
      Data Release Production
    • Urgent?:
      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

            Hide
            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.

            Show
            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.
            Hide
            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

            Show
            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
            Hide
            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. Nate Lust) give the meas_algorightms branch another look?

            New Jenkins + ci_hsc is running.

            Show
            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. Nate Lust ) give the meas_algorightms branch another look? New Jenkins + ci_hsc is running.
            Hide
            nlust Nate Lust added a comment -

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

            Show
            nlust Nate Lust added a comment - Thanks for getting that all turned around so fast, looks good to go to me
            Hide
            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.

            Show
            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.
            Hide
            nlust Nate Lust added a comment -

            Sounds good!

            Show
            nlust Nate Lust added a comment - Sounds good!
            Hide
            lauren Lauren MacArthur added a comment -

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

            Show
            lauren Lauren MacArthur added a comment - Final Jenkins passed...merged to master and done!

              People

              Assignee:
              lauren Lauren MacArthur
              Reporter:
              lauren Lauren MacArthur
              Reviewers:
              Nate Lust
              Watchers:
              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.