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

Enable rectangular binSizes in SubtractBackgroundTask

    Details

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

      Description

      While validating on HSC data, we encountered a use case for rectangular bin sizes in SubtractBackgroundTask. We propose two new config fields:

      • binSizeX
      • binSizeY

      By default they will be None and populated with the value in binSize. This change will be 100% backwards compatible. Everyone's config files will produce the same results.

        Attachments

          Issue Links

            Activity

            Hide
            yusra Yusra AlSayyad added a comment -

            One non-obvious decision is what users will expect now for polynomial approximation if they set:
            config.undersampleStyle == "INCREASE_NXNYSAMPLE"

            Would they expect the (a) largest square bins that produce enough points to fit a particular order or (b) the largest rectangular bins to produce enough points (shrinking only the side of the bin that was too large).

            Show
            yusra Yusra AlSayyad added a comment - One non-obvious decision is what users will expect now for polynomial approximation if they set: config.undersampleStyle == "INCREASE_NXNYSAMPLE" Would they expect the (a) largest square bins that produce enough points to fit a particular order or (b) the largest rectangular bins to produce enough points (shrinking only the side of the bin that was too large).
            Hide
            yusra Yusra AlSayyad added a comment -

            Paul, do you have time to review?

            Show
            yusra Yusra AlSayyad added a comment - Paul, do you have time to review?
            Hide
            price Paul Price added a comment -

            This looks great!

            I love that you cleaned up the order stuff, but maybe that should be on a separate commit.

            Can you explain in the commit message why Akima should be the default?

            Show
            price Paul Price added a comment - This looks great! I love that you cleaned up the order stuff, but maybe that should be on a separate commit. Can you explain in the commit message why Akima should be the default?
            Hide
            yusra Yusra AlSayyad added a comment -

            Merged

            Show
            yusra Yusra AlSayyad added a comment - Merged

              People

              • Assignee:
                yusra Yusra AlSayyad
                Reporter:
                yusra Yusra AlSayyad
                Reviewers:
                Paul Price
                Watchers:
                Jim Bosch, Paul Price, Yusra AlSayyad
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel