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

Cleanup piff PSF determiner model size config options

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_extensions_piff
    • Labels:
      None
    • Story Points:
      2
    • Sprint:
      AP S22-5 (April)
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      meas_extensions_piff sets the kernelSize parameter in config.setDefaults, but it is not a configurable parameter it has a different meaning in the parent class vs. in piffPsfDeterminer. For DM-34254 (and certainly other things), we need to be able to configure the psf model size. Given the min/max values also used in setDefaults, this is clearly a RangeField. The min/max options in the parent class don't make sense for piff, as they relate to the psf size calculated from the stellar moments.

      For this ticket, I'll make it configurable but keep the current default value of 21.

        Attachments

          Issue Links

          blocks

          Bug - A problem which impairs or prevents the functions of the product. DM-34254 ap_verify metrics report 0 completeness after switch to Piff

          • Done
          is triggered by

          Bug - A problem which impairs or prevents the functions of the product. DM-34254 ap_verify metrics report 0 completeness after switch to Piff

          • Done
          is triggering

          RFC - An issue whose primary purpose is to advertise an intent and allow stakeholders to comment. RFC-861 Clean up PsfDeterminer configs

          • Implemented
          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-34698 Default piff kernelSize to 25

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/36441/pipeline

            Show
            Parejkoj John Parejko added a comment - Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/36441/pipeline
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            Joshua Meyers: do you want to take a look? I believe I kept all the functionality, just made kernelSize a configurable field. I don't know if my docstring is correct though.

            Show
            Parejkoj John Parejko added a comment - Joshua Meyers : do you want to take a look? I believe I kept all the functionality, just made kernelSize a configurable field. I don't know if my docstring is correct though.
            Hide
            Permalink
            jmeyers314 Joshua Meyers added a comment -

            Isn't this already a configurable field via the base class? https://github.com/lsst/meas_algorithms/blob/main/python/lsst/meas/algorithms/psfDeterminer.py#L32

             

            Agree that it could be a RangeField though.

            Show
            jmeyers314 Joshua Meyers added a comment - Isn't this already a configurable field via the base class? https://github.com/lsst/meas_algorithms/blob/main/python/lsst/meas/algorithms/psfDeterminer.py#L32   Agree that it could be a RangeField though.
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            I just noted that `BasePsfDeterminerTask` requires a `config` arg as its first arg, but that is not a requirement of the API for `Task`s, and in fact `Task` behaves correctly if `config is None`, by creating a default config.

            Show
            Parejkoj John Parejko added a comment - I just noted that `BasePsfDeterminerTask` requires a `config` arg as its first arg, but that is not a requirement of the API for `Task`s, and in fact `Task` behaves correctly if `config is None`, by creating a default config.
            Hide
            Permalink
            Parejkoj John Parejko added a comment - - edited

            Post cleanup Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/36743/pipeline

            Show
            Parejkoj John Parejko added a comment - - edited Post cleanup Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/36743/pipeline
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            Handing this off to Arun Kannawadi to finish up: this probably requires an RFC, to deprecate those configs for piff.

            Show
            Parejkoj John Parejko added a comment - Handing this off to Arun Kannawadi to finish up: this probably requires an RFC, to deprecate those configs for piff.
            Hide
            Permalink
            kannawad Arun Kannawadi added a comment -

            I doubt we want to merge the piff PR (https://github.com/lsst/meas_extensions_piff/pull/9) associated with the ticket. The pex_config PR (https://github.com/lsst/pex_config/pull/71) is already merged, so we should perhaps mark it as Done.

            Show
            kannawad Arun Kannawadi added a comment - I doubt we want to merge the piff PR ( https://github.com/lsst/meas_extensions_piff/pull/9 ) associated with the ticket. The pex_config PR ( https://github.com/lsst/pex_config/pull/71 ) is already merged, so we should perhaps mark it as Done.

              People

              Assignee:
              kannawad Arun Kannawadi
              Reporter:
              Parejkoj John Parejko
              Reviewers:
              Joshua Meyers
              Watchers:
              Arun Kannawadi, Eric Bellm, Ian Sullivan, John Parejko, Joshua Meyers, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.