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

Add Psf-matched CTEs and Coadds as independent data products in DRP

    XMLWordPrintable

Details

    • 8
    • DRP S17-1, DRP S17-2, DRP S17-3, DRP S17-4, DRP S17-5, DRP S17-6
    • Data Release Production

    Description

      This ticket will incorporate psf-matched coadds into DRP and include the following:

      1) Develop plan for processing Psf-Matched coadds as part of DRP including interface in pipe_tasks commandline tasks, and butler data product naming convention and mapped filesystem paths: https://community.lsst.org/t/who-are-the-stakeholders-for-psf-matched-coadds/1439
      2) RFC plan.
      3) Add new data products to appropriate mapper files, including getting input on ones with unique file paths like obs_subaru and obs_sdss.
      4) Edit command line tasks (currently WarpAndPsfMatch, MakeCoaddTempExp, AssembleCoadd) to treat psf-matched and non-psf-matched coadds as separate entities.

      Attachments

        Issue Links

          Activity

            Yes, I share that feeling.

            yusra Yusra AlSayyad added a comment - Yes, I share that feeling.

            For the record, and just to make sure yusra doesn't catch any flak —

            We discussed review procedures at the 2017-04-06 standup, partly in reference to this ticket, and partly because I've given misleading advice in the past.

            Here, the word of the Developer Guide is law. In particular, while a review is still iterating, the ticket stays in status "in review". The reviewer should only mark a ticket as "reviewed" when they are satisfied that all their comments have been addressed or will be addressed: it's effectively permission to merge at the developer's discretion.

            Note that we do not give any real weight to the classification assigned to reviews on GitHub PRs. In particular, if a ticket is marked in JIRA as "reviewed" but on GitHub as "request changes", them the JIRA status is the only one that matters.

            That's relevant in this case, because this ticket is marked as "reviewed" but the PR on pipe_tasks is still marked as "request changes". Formally, I reckon that means Yusra can merge this whenever she's ready without further sign-off.

            swinbank John Swinbank added a comment - For the record, and just to make sure yusra doesn't catch any flak — We discussed review procedures at the 2017-04-06 standup, partly in reference to this ticket, and partly because I've given misleading advice in the past. Here, the word of the Developer Guide is law. In particular, while a review is still iterating, the ticket stays in status "in review". The reviewer should only mark a ticket as "reviewed" when they are satisfied that all their comments have been addressed or will be addressed: it's effectively permission to merge at the developer's discretion. Note that we do not give any real weight to the classification assigned to reviews on GitHub PRs. In particular, if a ticket is marked in JIRA as "reviewed" but on GitHub as "request changes", them the JIRA status is the only one that matters. That's relevant in this case, because this ticket is marked as "reviewed" but the PR on pipe_tasks is still marked as "request changes". Formally, I reckon that means Yusra can merge this whenever she's ready without further sign-off.

            OK, this is finally ready to go. price, I made all changes requested except:

            • Your suggestion to squash the two commits for (1) adding PSF-Matched data products and (2) changing the name of coadd temp exps to warps per RFC-283. From my perspective, it's already confusing that they are on the same ticket. Putting them on separate commits was a way to isolate them in case we need to revert one and not the other.
            • MakeCoaddTempExp: whereas I agree with the idea, I didn't move the contents of the try block inside createTempExp to its own method now. There are 5 variables that are updated with every calexp (coaddTempExps, totGoodPix,numGoodPix, inputRecorder, didSetMetadata). With the current design, the new method would take 10 arguments. We could refactor this into processVisit() which then calls processCalexp() which updates a container of holding the coaddTempExps, totGoodPix,numGoodPix, inputRecorder, didSetMetadata. Outside the scope of this ticket but doable on DM-9610.

            Major changes since first iteration:

            • AssembleCoadd only makes one warp type coadd at a time
            • PSF-Matched coadds are generated and tested in testCoadds.py
            • WarpType is now an Enum
            • createTempExp returns a Struct of a dict called "exposures" whose keys are one or both of WarpType:DIRECT and WarpType:PSF_MATCHED

            Given that there's significant new stuff here since the original review, at this point I think I'd prefer if someone looked over it quickly.

            yusra Yusra AlSayyad added a comment - OK, this is finally ready to go. price , I made all changes requested except: Your suggestion to squash the two commits for (1) adding PSF-Matched data products and (2) changing the name of coadd temp exps to warps per RFC-283 . From my perspective, it's already confusing that they are on the same ticket. Putting them on separate commits was a way to isolate them in case we need to revert one and not the other. MakeCoaddTempExp: whereas I agree with the idea, I didn't move the contents of the try block inside createTempExp to its own method now. There are 5 variables that are updated with every calexp (coaddTempExps, totGoodPix,numGoodPix, inputRecorder, didSetMetadata). With the current design, the new method would take 10 arguments. We could refactor this into processVisit() which then calls processCalexp() which updates a container of holding the coaddTempExps, totGoodPix,numGoodPix, inputRecorder, didSetMetadata. Outside the scope of this ticket but doable on DM-9610 . Major changes since first iteration: AssembleCoadd only makes one warp type coadd at a time PSF-Matched coadds are generated and tested in testCoadds.py WarpType is now an Enum createTempExp returns a Struct of a dict called "exposures" whose keys are one or both of WarpType:DIRECT and WarpType:PSF_MATCHED Given that there's significant new stuff here since the original review, at this point I think I'd prefer if someone looked over it quickly.

            Given that there's significant new stuff here since the original review, at this point I think I'd prefer if someone looked over it quickly.

            Given that price is already familiar with the work, I guess it makes sense for him to do it? I believe he'll be back from wombatting mid-next-week — will that be soon enough for you?

            swinbank John Swinbank added a comment - Given that there's significant new stuff here since the original review, at this point I think I'd prefer if someone looked over it quickly. Given that price is already familiar with the work, I guess it makes sense for him to do it? I believe he'll be back from wombatting mid-next-week — will that be soon enough for you?

            I'm going to merge this after three Jenkins configs pass – to double checking nothing serious has changed since April.

            yusra Yusra AlSayyad added a comment - I'm going to merge this after three Jenkins configs pass – to double checking nothing serious has changed since April.

            People

              yusra Yusra AlSayyad
              yusra Yusra AlSayyad
              Paul Price
              John Swinbank, Paul Price, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.