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

Update obs package use of filter.getId() in _computeCoaddExposureId

    XMLWordPrintable

    Details

    • Story Points:
      2
    • Urgent?:
      No

      Description

      This may be a "won't fix" because it only affects the gen2 CameraMappers, but lsst, subaru, decam, and cfht all use filter.getId() in their calculation of the singleFilter coadd exposure id. The new FilterLabel does not have a numeric Id, so these mappers will have to be changed. However, I do not know what the time ordering of gen2 removal vs. Filter removal is: if gen2 is removed first, then this won't matter at all.

      I did not change this on DM-27170 because I did not want to alter this calculation in gen2 without more discussion, and want to get that ticket finished up.

        Attachments

          Issue Links

          blocks

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-27177 Remove lsst.afw.image.Filter

          • Done
          is triggered by

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-27170 Deprecate and phase out afw::image::Filter

          • Done

            Activity

            Ascending order - Click to sort in descending order
            No builds found.
            Parejkoj John Parejko created issue -
            Parejkoj John Parejko made changes -
            Field Original Value New Value
            Link This issue is triggered by DM-27170 [ DM-27170 ]
            Parejkoj John Parejko made changes -
            Link This issue blocks DM-27177 [ DM-27177 ]
            sullivan Ian Sullivan made changes -
            Labels SciencePipelines
            krzys Krzysztof Findeisen made changes -
            Labels SciencePipelines SciencePipelines gen2-only
            Parejkoj John Parejko made changes -
            Watchers Ian Sullivan, Jim Bosch, John Parejko, Krzysztof Findeisen, Tim Jenness [ Ian Sullivan, Jim Bosch, John Parejko, Krzysztof Findeisen, Tim Jenness ] Ian Sullivan, Jim Bosch, John Parejko, Krzysztof Findeisen, Tim Jenness, Yusra AlSayyad [ Ian Sullivan, Jim Bosch, John Parejko, Krzysztof Findeisen, Tim Jenness, Yusra AlSayyad ]
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment -

            Pinging this, as it's an indirect blocker for DM-29437.

            Show
            krzys Krzysztof Findeisen added a comment - Pinging this, as it's an indirect blocker for DM-29437 .
            Hide
            Permalink
            lauren Lauren MacArthur added a comment -

            I'm trying to address this on DM-30284 (no absolute promises on strict backwards-compatibility, but I don't think anybody cares for gen2!) I also have only been looking at obs_lsst and obs_subaru so far, but, presumably, if I do manage to fix those two, any other obs_package will be able to follow(-ish) suit.

            Show
            lauren Lauren MacArthur added a comment - I'm trying to address this on DM-30284 (no absolute promises on strict backwards-compatibility, but I don't think anybody cares for gen2!) I also have only been looking at obs_lsst and obs_subaru so far, but, presumably, if I do manage to fix those two, any other obs_package will be able to follow(-ish) suit.
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            Should we close this as "Won't Fix" since it only affects gen2? I think we agreed that we can't remove `Filter` until we've cut out all the gen2 code, but this is one of the two direct blocker tickets for removing Filter. I was reminded of all this when building meas_algorithms and seeing the huge number of deprecation warnings from Filter.

            Show
            Parejkoj John Parejko added a comment - Should we close this as "Won't Fix" since it only affects gen2? I think we agreed that we can't remove `Filter` until we've cut out all the gen2 code, but this is one of the two direct blocker tickets for removing Filter. I was reminded of all this when building meas_algorithms and seeing the huge number of deprecation warnings from Filter.
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            Closing as Won't Fix.

            Show
            jbosch Jim Bosch added a comment - Closing as Won't Fix.
            jbosch Jim Bosch made changes -
            Resolution Done [ 10000 ]
            Status To Do [ 10001 ] Won't Fix [ 10405 ]
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment - - edited

            I'm trying to implement DM-27177 and have just run into this issue in obs.lsst.LsstCamBaseMapper._computeCoaddExposureId. How should I handle it? Should I just have _computeCoaddExposureId raise NotImplementedError? Should I remove the if singleFilter block? Hash the filter name?

            I have no idea what will break if, for example, the IDs are no longer unique.

            Show
            krzys Krzysztof Findeisen added a comment - - edited I'm trying to implement DM-27177 and have just run into this issue in obs.lsst.LsstCamBaseMapper._computeCoaddExposureId . How should I handle it? Should I just have _computeCoaddExposureId raise NotImplementedError ? Should I remove the if singleFilter block? Hash the filter name? I have no idea what will break if, for example, the IDs are no longer unique.
            Hide
            Permalink
            tjenness Tim Jenness added a comment -

            I'm about to merge DM-34919 (as soon as the jira ticket is marked reviewed) and that will delete this code.

            Show
            tjenness Tim Jenness added a comment - I'm about to merge DM-34919 (as soon as the jira ticket is marked reviewed) and that will delete this code.
            Hide
            Permalink
            tjenness Tim Jenness added a comment -

            Krzysztof Findeisen That code should no longer exist.

            Show
            tjenness Tim Jenness added a comment - Krzysztof Findeisen That code should no longer exist.
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment -

            Confirmed, but I had to deal with something similar in obs_cfht. I took the hash approach; so far it's passing tests.

            Show
            krzys Krzysztof Findeisen added a comment - Confirmed, but I had to deal with something similar in obs_cfht . I took the hash approach; so far it's passing tests.
            Hide
            Permalink
            tjenness Tim Jenness added a comment -

            I also have a ticket in review to remove the code from obs_cfht...

            Show
            tjenness Tim Jenness added a comment - I also have a ticket in review to remove the code from obs_cfht...

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              Parejkoj John Parejko
              Watchers:
              Ian Sullivan, Jim Bosch, John Parejko, Krzysztof Findeisen, Lauren MacArthur, Tim Jenness, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.