Details

    • Type: Technical task
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_base
    • Labels:
      None
    • Story Points:
      2
    • Sprint:
      Science Pipelines DM-S15-1, Science Pipelines DM-S15-2
    • Team:
      Data Release Production

      Description

      Provide calibration transforms for algorithms measuring shapes.

        Attachments

          Activity

          Hide
          swinbank John Swinbank added a comment -

          Hi Jim,

          As discussed, would you mind taking a look at this? The basic mechanism is very similar to DM-2306, but dealing with shapes rather than centroids. However, there are two things which I'd appreciate your feedback on:

          • I've adapted the ShapeResultKey to handle either pixel (x,y) or celestial (RA, dec) coordinates. This is slightly clunky, because it still uses x and y as variable names internally, and it means a change to the external interface. I'm certainly not wedded to this approach, and if you have a better suggestion please let me know.
          • I added a free function for generating the transformation matrix required for transforming shapes. This is quite similar to functionality that's already available in afw::geom. I wondered about going for closer integration here, but decided I preferred the simplicity of the free function in meas_base. You have a better overview of other likely users of this code, though, so I would be happy to rethink this if you think it's worthwhile.

          Anyway, the code is on tickets/DM-2307 in afw, meas_base and meas_modelfit.

          Show
          swinbank John Swinbank added a comment - Hi Jim, As discussed, would you mind taking a look at this? The basic mechanism is very similar to DM-2306 , but dealing with shapes rather than centroids. However, there are two things which I'd appreciate your feedback on: I've adapted the ShapeResultKey to handle either pixel (x,y) or celestial (RA, dec) coordinates. This is slightly clunky, because it still uses x and y as variable names internally, and it means a change to the external interface. I'm certainly not wedded to this approach, and if you have a better suggestion please let me know. I added a free function for generating the transformation matrix required for transforming shapes. This is quite similar to functionality that's already available in afw::geom . I wondered about going for closer integration here, but decided I preferred the simplicity of the free function in meas_base . You have a better overview of other likely users of this code, though, so I would be happy to rethink this if you think it's worthwhile. Anyway, the code is on tickets/ DM-2307 in afw , meas_base and meas_modelfit .
          Hide
          jbosch Jim Bosch added a comment -

          Review complete. There are a few minor comments on the PRs. The only big one is that I think we should continue to use "xx, yy, xy" for even celestial QuadrupleKey fields, but with units and documentation updated to indicate that we mean x=ra, y=dec. I think that should allow some of this to be simplified a bit, but more importantly I think it matches standard practice.

          Show
          jbosch Jim Bosch added a comment - Review complete. There are a few minor comments on the PRs. The only big one is that I think we should continue to use "xx, yy, xy" for even celestial QuadrupleKey fields, but with units and documentation updated to indicate that we mean x=ra, y=dec. I think that should allow some of this to be simplified a bit, but more importantly I think it matches standard practice.
          Hide
          swinbank John Swinbank added a comment -

          Thanks for the review – changes made as suggested and merged to master.

          Show
          swinbank John Swinbank added a comment - Thanks for the review – changes made as suggested and merged to master.

            People

            • Assignee:
              swinbank John Swinbank
              Reporter:
              swinbank John Swinbank
              Reviewers:
              Jim Bosch
              Watchers:
              Jim Bosch, John Swinbank
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel