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

Fixup dimensions on ProcessCcdWithFakesTask/MatchFakesTask

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • pipe_tasks
    • None
    • 6
    • AP F21-6 (November), AP S22-1 (December)
    • Alert Production
    • No

    Description

      ProcessCcdWithFakes and MatchFakesTask currently have dimensions of [skymap, tract, instrument, visit, detector]. This causes processing problems for data multi-tract data as the inputs with tract are ambiguous and a quantum graph can not be made.

      After chatting with jbosch and sophiereed, here's the solution to be worked on in this ticket:

      • Remove tract from the two tasks dimensions (both tasks output as [instrument, visit, detector]).
      • Make any tract dependent input use the multi and deferLoad options.
      • Write code to decide which tract to use based on which tract the detector visit is nearest. For the fakes catalog, write code to concatenate the fakes catalog together without duplicating data.

      Attachments

        Issue Links

          Activity

            cmorrison Chris Morrison [X] (Inactive) added a comment - Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35488/pipeline

            Hey Lee, let me know what ya think and feel free to loop in Sophie.

            cmorrison Chris Morrison [X] (Inactive) added a comment - Hey Lee, let me know what ya think and feel free to loop in Sophie.
            lskelvin Lee Kelvin added a comment -

            Hey Chris, I've left some comments on your various PRs. All in all, this looks to be in pretty good shape. Other than Jenkins, have you ran this through on some test data to confirm that the results are consistent with prior outputs?

            I note that Surhud More also left some good comments on deprecated names and a typo, which would also be useful to look at.

            I am concerned about renaming the primary synthetic source injection task label in obs_subaru and ci_hsc_gen3 from processCcdWithFakes to detectorVisitFakes. The new name, whilst consistent with ap_verify, is not in keeping with the current DRP naming convention, nor is it particularly descriptive. My suggestion, if this label must be changed, is to change it to something like singleFrameWithFakes. It would be good to get sophiereed's thoughts on this too.

            lskelvin Lee Kelvin added a comment - Hey Chris, I've left some comments on your various PRs. All in all, this looks to be in pretty good shape. Other than Jenkins, have you ran this through on some test data to confirm that the results are consistent with prior outputs? I note that Surhud More also left some good comments on deprecated names and a typo, which would also be useful to look at. I am concerned about renaming the primary synthetic source injection task label in obs_subaru and ci_hsc_gen3 from processCcdWithFakes to detectorVisitFakes . The new name, whilst consistent with ap_verify , is not in keeping with the current DRP naming convention, nor is it particularly descriptive. My suggestion, if this label must be changed, is to change it to something like singleFrameWithFakes . It would be good to get sophiereed 's thoughts on this too.
            sophiereed Sophie Reed added a comment -

            I agree with Lee about naming, detectorVisitFakes doesn't really give me any indication of what is happening or a hint that processing beyond just addition of fakes is going on. I would second Lee's suggestion.

            sophiereed Sophie Reed added a comment - I agree with Lee about naming, detectorVisitFakes doesn't really give me any indication of what is happening or a hint that processing beyond just addition of fakes is going on. I would second Lee's suggestion.
            lskelvin Lee Kelvin added a comment -

            Thanks again Chris for the edits made on GitHub. Assuming test runs of these scripts have proceeded through without issue, then I think this looks good to merge to me - very nicely done!

            lskelvin Lee Kelvin added a comment - Thanks again Chris for the edits made on GitHub. Assuming test runs of these scripts have proceeded through without issue, then I think this looks good to merge to me - very nicely done!
            cmorrison Chris Morrison [X] (Inactive) added a comment - Final Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35589/pipeline

            People

              cmorrison Chris Morrison [X] (Inactive)
              cmorrison Chris Morrison [X] (Inactive)
              Lee Kelvin
              Chris Morrison [X] (Inactive), Eric Bellm, Ian Sullivan, Lee Kelvin, Meredith Rawls, Sophie Reed
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.