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

Implement new CalibrateImageTask

    XMLWordPrintable

Details

    • 24
    • AP F23-1 (June), AP F23-2 (July)
    • Alert Production
    • No

    Description

      Assist erykoff with refactoring the existing image characterization and calibration tasks into a single unified CalibrateImageTask, following the design laid out in DM-37584.

      Some relevant design notes from jbosch in this google doc:
      https://docs.google.com/document/d/1V0DYlKbj7BVIguDI_Jlmh6zI5Q1DeW8nZXPKLO7SY8w/edit

      Further design notes in the February 2023 update branch of DMTN-172

      Attachments

        Issue Links

          Activity

            Parejkoj John Parejko added a comment - - edited

            Packages modified on this ticket:

            Parejkoj John Parejko added a comment - - edited Packages modified on this ticket: pipe_tasks (where the new task is): https://github.com/lsst/pipe_tasks/pull/802 meas_base (improvements to TestDataset): https://github.com/lsst/meas_base/pull/247 meas_algorithms (new refcat mocker and source selector bugfix): https://github.com/lsst/meas_algorithms/pull/338 meas_astrom (better error handling of small datasets and disabling source selectors): https://github.com/lsst/meas_astrom/pull/181 afw (tweaks to exposure apCorrMap interface): https://github.com/lsst/afw/pull/693

            This pipeline task config worked to let me run ap_verify DC2 data through the new task. I added it to an ApVerify.yaml file from a previous run.

            tasks:
              calibrateImage:
                class: lsst.pipe.tasks.calibrateImage.CalibrateImageTask
                config:
                  connections.astrometry_ref_cat: "cal_ref_cat_2_2"
                  connections.photometry_ref_cat: "cal_ref_cat_2_2"
                  astrometry_ref_loader.anyFilterMapsToThis: None
                  python: >
                    config.astrometry_ref_loader.filterMap = {band: 'lsst_%s_smeared' % (band) for band in 'ugrizy'};
                    config.photometry_ref_loader.filterMap = {band: 'lsst_%s_smeared' % (band) for band in 'ugrizy'};
                  psf_detection.background.approxOrderX: 1
                  psf_detection.tempLocalBackground.approxOrderX: 1
                  psf_detection.tempWideBackground.approxOrderX: 1
                  psf_repair.cosmicray.background.approxOrderX: 1
                  psf_detection.isotropicGrow: True
            

            Parejkoj John Parejko added a comment - This pipeline task config worked to let me run ap_verify DC2 data through the new task. I added it to an ApVerify.yaml file from a previous run. tasks: calibrateImage: class: lsst.pipe.tasks.calibrateImage.CalibrateImageTask config: connections.astrometry_ref_cat: "cal_ref_cat_2_2" connections.photometry_ref_cat: "cal_ref_cat_2_2" astrometry_ref_loader.anyFilterMapsToThis: None python: > config.astrometry_ref_loader.filterMap = {band: 'lsst_%s_smeared' % (band) for band in 'ugrizy'}; config.photometry_ref_loader.filterMap = {band: 'lsst_%s_smeared' % (band) for band in 'ugrizy'}; psf_detection.background.approxOrderX: 1 psf_detection.tempLocalBackground.approxOrderX: 1 psf_detection.tempWideBackground.approxOrderX: 1 psf_repair.cosmicray.background.approxOrderX: 1 psf_detection.isotropicGrow: True
            Parejkoj John Parejko added a comment - Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/39082/pipeline

            jbosch: Thanks for volunteering to review while erykoff is on vacation. No rush on the review: it's ~1300 lines total. The changes outside of the new task and its tests should all be pretty straightforward: they were things that helped me write the new task and test it, or were small fixes I did along the way. The commits should all be fully atomic, so you can review them independently if that helps with context.

            I left quite a few TODOs in the new task and its tests: I hope that we can answer most of them during the review, and whatever is left we can file tickets for (some I'll have a ticket for once I get the config defaults RFC finalized).

            Parejkoj John Parejko added a comment - jbosch : Thanks for volunteering to review while erykoff is on vacation. No rush on the review: it's ~1300 lines total. The changes outside of the new task and its tests should all be pretty straightforward: they were things that helped me write the new task and test it, or were small fixes I did along the way. The commits should all be fully atomic, so you can review them independently if that helps with context. I left quite a few TODOs in the new task and its tests: I hope that we can answer most of them during the review, and whatever is left we can file tickets for (some I'll have a ticket for once I get the config defaults RFC finalized).
            jbosch Jim Bosch added a comment -

            Most of my PR comments are about existing TODO comments, and I haven't tried at all to enforce that TODOs have ticket references.  Some changes are definitely needed before this becomes the default (in many cases due to TODOs already present in the code), but I have no problem with merging this pretty much as-is and addressing however much of that as you'd like later.

            Certainly a lot of those TODOs can only be addressed by actually experimenting with this task on real data (often significant amounts of real data), and that'll be much easier after this has been merged to main.

             

            jbosch Jim Bosch added a comment - Most of my PR comments are about existing TODO comments, and I haven't tried at all to enforce that TODOs have ticket references.  Some changes are definitely needed before this becomes the default (in many cases due to TODOs already present in the code), but I have no problem with merging this pretty much as-is and addressing however much of that as you'd like later. Certainly a lot of those TODOs can only be addressed by actually experimenting with this task on real data (often significant amounts of real data), and that'll be much easier after this has been merged to main .  

            People

              Parejkoj John Parejko
              sullivan Ian Sullivan
              Jim Bosch
              Eli Rykoff, Ian Sullivan, Jim Bosch, John Parejko, Merlin Fisher-Levine
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.