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

    • Story Points:
      8
    • Sprint:
      DRP S17-1, DRP S17-2, DRP S17-3, DRP S17-4, DRP S17-5, DRP S17-6
    • Team:
      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

            Hide
            yusra Yusra AlSayyad added a comment -

            Yes, I share that feeling.

            Show
            yusra Yusra AlSayyad added a comment - Yes, I share that feeling.
            Hide
            swinbank John Swinbank added a comment -

            For the record, and just to make sure Yusra AlSayyad 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.

            Show
            swinbank John Swinbank added a comment - For the record, and just to make sure Yusra AlSayyad 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.
            Hide
            yusra Yusra AlSayyad added a comment -

            OK, this is finally ready to go. Paul 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.

            Show
            yusra Yusra AlSayyad added a comment - OK, this is finally ready to go. Paul 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.
            Hide
            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 Paul 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?

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

            Show
            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

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

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins Builds

                  No builds found.