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

Improve MaxPsfWcsSelectImagesTask

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: Alert Production, ap_pipe
    • Labels:
      None
    • Story Points:
      4
    • Epic Link:
    • Sprint:
      Alert Production F17 - 10, Alert Production F17 - 11, AP S18-1, AP S18-2, AP S18-3, AP S18-4, AP S18-5, AP S18-6
    • Team:
      Alert Production

      Description

      The ImageSelector subclass imported in https://jira.lsstcorp.org/browse/DM-10977 requires manually specifying minimum and maximum FWHMs for constructing a "best seeing" coadd. This is limiting for several reasons and requires manual offline analysis to determine those numbers.

      This ticket is to change the algorithm to automatically determine the N best-seeing images in a patch for coaddition.

        Attachments

          Issue Links

          blocks

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-14260 Re-run HiTS data processing with current ap_pipe

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-14580 Create tests for BestSeeingWcsSelectImagesTask.

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            ebellm Eric Bellm added a comment -

            Krzysztof, can you take a look at this? Note that I have deferred the unit tests to DM-14580.

            Show
            ebellm Eric Bellm added a comment - Krzysztof, can you take a look at this? Note that I have deferred the unit tests to DM-14580 .
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment - - edited

            I'd like to see the ap_pipe commits double-checked before this gets merged – they seem to be mixed up quite a bit at the moment. I also found a bug in how min/max FWHM is handled that makes me think we should require tests after all (unless Meredith Rawls is ok dealing with potentially very low quality output).

            Show
            krzys Krzysztof Findeisen added a comment - - edited I'd like to see the ap_pipe commits double-checked before this gets merged – they seem to be mixed up quite a bit at the moment. I also found a bug in how min/max FWHM is handled that makes me think we should require tests after all (unless Meredith Rawls is ok dealing with potentially very low quality output).
            Hide
            Permalink
            mrawls Meredith Rawls added a comment -

            I think I'm OK with Eric Bellm's stated goal of getting this in place now so we can generate new templates in a finite timescale at all, and adding a test later (as in, up next, very important and soon, but still later). I'd be happy to be part of a pair coding exercise with either of you to write some tests for this specifically and/or for ap_pipe in general, because they are lacking all around.

            Show
            mrawls Meredith Rawls added a comment - I think I'm OK with Eric Bellm 's stated goal of getting this in place now so we can generate new templates in a finite timescale at all, and adding a test later (as in, up next, very important and soon, but still later). I'd be happy to be part of a pair coding exercise with either of you to write some tests for this specifically and/or for ap_pipe in general, because they are lacking all around.
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment -

            Meredith Rawls so you can't use the old MaxPsfWcsSelectImagesTask in the meantime?

            Show
            krzys Krzysztof Findeisen added a comment - Meredith Rawls so you can't use the old MaxPsfWcsSelectImagesTask in the meantime?
            Hide
            Permalink
            ebellm Eric Bellm added a comment -

            Krzysztof Findeisen that requires some manual analysis and is using effectively the same code being committed here. It didn't have tests either...

            Show
            ebellm Eric Bellm added a comment - Krzysztof Findeisen that requires some manual analysis and is using effectively the same code being committed here. It didn't have tests either...
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment -

            Fine. I'll approve the review once we're sure the right changes are being committed to ap_pipe.

            Show
            krzys Krzysztof Findeisen added a comment - Fine. I'll approve the review once we're sure the right changes are being committed to ap_pipe .
            Hide
            Permalink
            ebellm Eric Bellm added a comment -

            Krzysztof Findeisen, can you take another look?

            Show
            ebellm Eric Bellm added a comment - Krzysztof Findeisen , can you take another look?
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment -

            No further objections.

            Show
            krzys Krzysztof Findeisen added a comment - No further objections.

              People

              Assignee:
              ebellm Eric Bellm
              Reporter:
              ebellm Eric Bellm
              Reviewers:
              Krzysztof Findeisen
              Watchers:
              Eric Bellm, Krzysztof Findeisen, Meredith Rawls
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.