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

Port HSC-1199 to LSST (UNMASKEDNAN mask propagates to all amplifiers)

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: ip_isr
    • Labels:
      None

      Description

      Port issue HSC-1199 to LSST stack to address UNMASKEDNAN mask propagates to all amplifiers

        Attachments

          Activity

          No builds found.
          nlust Nate Lust created issue -
          nlust Nate Lust made changes -
          Field Original Value New Value
          Epic Link DM-1942 [ 16008 ]
          Hide
          nlust Nate Lust added a comment -

          commit 768e2f8db45d18811ea554327b8ad7a92989fa60 on the HSC side is unneeded because the change was already made in commit ecde9a6f869d306310a34fdc9ae72cc419ea9a2f on the LSST side

          Show
          nlust Nate Lust added a comment - commit 768e2f8db45d18811ea554327b8ad7a92989fa60 on the HSC side is unneeded because the change was already made in commit ecde9a6f869d306310a34fdc9ae72cc419ea9a2f on the LSST side
          swinbank John Swinbank made changes -
          Summary Port HSC-1199 to LSST Port HSC-1199 to LSST (UNMASKEDNAN mask propagates to all amplifiers)
          swinbank John Swinbank made changes -
          Team Data Release Production [ 10301 ]
          nlust Nate Lust made changes -
          Status To Do [ 10001 ] In Progress [ 3 ]
          nlust Nate Lust made changes -
          Story Points 1 2
          Hide
          nlust Nate Lust added a comment -

          Successfully runs in local tests and buildbot. Once understanding the changes, it was strait forward. It was just complex as the two code bases have diverged

          Show
          nlust Nate Lust added a comment - Successfully runs in local tests and buildbot. Once understanding the changes, it was strait forward. It was just complex as the two code bases have diverged
          nlust Nate Lust made changes -
          Reviewers Lauren MacArthur [ lauren ]
          Status In Progress [ 3 ] In Review [ 10004 ]
          swinbank John Swinbank made changes -
          Sprint Science Pipelines DM-S15-4 [ 159 ]
          Hide
          lauren Lauren MacArthur added a comment -

          Could you expand a bit on what you mean by "successfully runs" and "local tests" (e.g. did you run pre- & post-fix on a problematic case using real data?)

          Show
          lauren Lauren MacArthur added a comment - Could you expand a bit on what you mean by "successfully runs" and "local tests" (e.g. did you run pre- & post-fix on a problematic case using real data?)
          Hide
          lauren Lauren MacArthur added a comment -

          I see you have inherited a non-squashed fixup! commit. This would have been worth noting & it is perhaps best to squash it.

          Show
          lauren Lauren MacArthur added a comment - I see you have inherited a non-squashed fixup! commit. This would have been worth noting & it is perhaps best to squash it.
          jbosch Jim Bosch made changes -
          Labels HSC
          Hide
          swinbank John Swinbank added a comment -

          I'm not completely sure if Lauren MacArthur's review is actually complete. Since she won't be back until nicely into the next sprint, let's carry this over to that.

          Show
          swinbank John Swinbank added a comment - I'm not completely sure if Lauren MacArthur 's review is actually complete. Since she won't be back until nicely into the next sprint, let's carry this over to that.
          swinbank John Swinbank made changes -
          Sprint Science Pipelines DM-S15-4 [ 159 ] Science Pipelines DM-S15-5 [ 162 ]
          Hide
          swinbank John Swinbank added a comment -

          Just spoke to Lauren – she confirms the review on this is done and Nate is just finishing tidying things up before merging.

          Show
          swinbank John Swinbank added a comment - Just spoke to Lauren – she confirms the review on this is done and Nate is just finishing tidying things up before merging.
          Hide
          swinbank John Swinbank added a comment -

          Actually... Lauren is going to take another look at the updated test suite once Nate is ready before he merges. Hence this really is still "in review",

          Show
          swinbank John Swinbank added a comment - Actually... Lauren is going to take another look at the updated test suite once Nate is ready before he merges. Hence this really is still "in review",
          swinbank John Swinbank made changes -
          Sprint Science Pipelines DM-S15-5 [ 162 ] Science Pipelines DM-S15-5, Science Pipelines DM-S15-6 [ 162, 165 ]
          swinbank John Swinbank made changes -
          Rank Ranked higher
          Hide
          nlust Nate Lust added a comment -

          Fix-up has been squashed. The changes have been run through buildbot, and the unittest suite independently. I was able to hack together code that would fail before the changes and pass after the changes. I have noted future changes which should be considered for overscan correction in DM-3589. I additionally added new documentation to make certain parts of the code more understandable for future developers.

          Show
          nlust Nate Lust added a comment - Fix-up has been squashed. The changes have been run through buildbot, and the unittest suite independently. I was able to hack together code that would fail before the changes and pass after the changes. I have noted future changes which should be considered for overscan correction in DM-3589 . I additionally added new documentation to make certain parts of the code more understandable for future developers.
          Hide
          lauren Lauren MacArthur added a comment -

          Looks good modulo a few minor typos in your last commit (comments on GitHub). Fix those and you are free to merge. Thanks for creating the other ticket.

          Show
          lauren Lauren MacArthur added a comment - Looks good modulo a few minor typos in your last commit (comments on GitHub). Fix those and you are free to merge. Thanks for creating the other ticket.
          lauren Lauren MacArthur made changes -
          Status In Review [ 10004 ] Reviewed [ 10101 ]
          Hide
          nlust Nate Lust added a comment -

          Merged to master

          Show
          nlust Nate Lust added a comment - Merged to master
          nlust Nate Lust made changes -
          Resolution Done [ 10000 ]
          Status Reviewed [ 10101 ] Done [ 10002 ]
          swinbank John Swinbank made changes -
          Story Points 2 4
          Hide
          swinbank John Swinbank added a comment -

          Could you add a brief note to https://confluence.lsstcorp.org/display/DM/Data+Release+Production+WIP+S15+release+notes describing what changed here, please? Thanks!

          Show
          swinbank John Swinbank added a comment - Could you add a brief note to https://confluence.lsstcorp.org/display/DM/Data+Release+Production+WIP+S15+release+notes describing what changed here, please? Thanks!
          Hide
          swinbank John Swinbank added a comment -

          Reminder to update the release notes!

          Show
          swinbank John Swinbank added a comment - Reminder to update the release notes!
          swinbank John Swinbank made changes -
          Labels HSC

            People

            Assignee:
            nlust Nate Lust
            Reporter:
            nlust Nate Lust
            Reviewers:
            Lauren MacArthur
            Watchers:
            John Swinbank, Lauren MacArthur, Nate Lust
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins Builds

                No builds found.