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

Refactor and unify CoaddPsf, CoaddBoundedField, and CoaddTransmissionCurve

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Won't Fix
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_algorithms
    • Labels:
      None
    • Story Points:
      4
    • Team:
      Data Release Production

      Description

      These components all do similar things but take different approaches to their implementation and public interface, making it hard to share much code between them.

      • CoaddPsf uses an internal ExposureCatalog, which makes both its persisted and in-memory sizes larger than they need to be, but simplifies its persistence and evaluation code.  CoaddBoundedField and CoaddTransmissionCurve (still WIP on DM-12370) use a vector of custom structs to avoid this.
      • CoaddPsf and CoaddBoundedField have public interfaces that are strongly tied to these implementation choices, while CoaddTransmissionCurve's implementation is entirely private (even the class itself is hidden behind a factory function).

      All three classes should probably use a vector a shared object (not ExposureCatalog, but also not vectors of custom structs) to consolidate persistence and lookup code.  They should also probably all be hidden behind private factory functions or at least take a consistent approach to this question that does not expose their implementations.

        Attachments

          Issue Links

            Activity

            No builds found.
            jbosch Jim Bosch created issue -
            jbosch Jim Bosch made changes -
            Field Original Value New Value
            Link This issue relates to DM-12370 [ DM-12370 ]
            tjenness Tim Jenness made changes -
            Remote Link This issue links to "Page (Confluence)" [ 31143 ]
            Hide
            tjenness Tim Jenness added a comment -

            Jim Bosch is this ticket still needed?

            Show
            tjenness Tim Jenness added a comment - Jim Bosch is this ticket still needed?
            Hide
            jbosch Jim Bosch added a comment -

            I'm calling this Won't Fix; there is indeed some mild duplication of logic here, but not enough that it's worth the probable disruption to fix (especially since all of this code has been stable and battle-tested for a while).  Hopefully future work on cell-based coadds will make this a moot point anyway.

            Show
            jbosch Jim Bosch added a comment - I'm calling this Won't Fix; there is indeed some mild duplication of logic here, but not enough that it's worth the probable disruption to fix (especially since all of this code has been stable and battle-tested for a while).  Hopefully future work on cell-based coadds will make this a moot point anyway.
            jbosch Jim Bosch made changes -
            Resolution Done [ 10000 ]
            Status To Do [ 10001 ] Won't Fix [ 10405 ]

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Watchers:
              Jim Bosch, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.