Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-312

Make PSFEx the default PSF estimator

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      A fair amount of the recent development on the measurement side has been done using PSFEx as the input PSF estimator. This means that testing is more complete with PSFEx than it is with the default, PCA, PSF estimator. It would be nice to support both, and in a perfect world both would work perfectly, but in reality, failures have been observed when using the PCA determiner that go away when PSFEx is used.

      I therefore request that we switch to using PSFEx as the default in all {{Task}}s. There already exists an epic to do this work, DM-5304. I do not know if the issues linked therein are still blocking issues, so it will take some triage to determine if that epic is still relevant.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            I believe this implies that meas_extensions_psfex will become a dependency of at least pipe_tasks? I'd be fine with that, but perhaps a bit worried if it needs to move lower than that in the package dependency list.

            Show
            jbosch Jim Bosch added a comment - I believe this implies that meas_extensions_psfex will become a dependency of at least pipe_tasks? I'd be fine with that, but perhaps a bit worried if it needs to move lower than that in the package dependency list.
            Hide
            rowen Russell Owen added a comment - - edited

            -1 until the tickets that block DM-5304 are dealt with and we have a plan for maintenance (a concern given all the bugs that have been found but not fixed).

            And yes at least some of those blockers are still relevant. I just stumbled across a new bug a few days ago and found that another of them was still present.

            Show
            rowen Russell Owen added a comment - - edited -1 until the tickets that block DM-5304 are dealt with and we have a plan for maintenance (a concern given all the bugs that have been found but not fixed). And yes at least some of those blockers are still relevant. I just stumbled across a new bug a few days ago and found that another of them was still present.
            Hide
            tjenness Tim Jenness added a comment -

            Simon Krughoff can this be adopted?

            Show
            tjenness Tim Jenness added a comment - Simon Krughoff can this be adopted?
            Hide
            krughoff Simon Krughoff added a comment -

            I was waiting because it sounded like there was some pushback. Russell Owen are you o.k. with this being adopted as long as we take care of the other blocking tickets before implementing the change of default?

            Show
            krughoff Simon Krughoff added a comment - I was waiting because it sounded like there was some pushback. Russell Owen are you o.k. with this being adopted as long as we take care of the other blocking tickets before implementing the change of default?
            Hide
            rowen Russell Owen added a comment - - edited

            I suggest the following as a minimum:

            • Add tests that catch the reported bugs
            • Fix the reported bugs
            • Take a cleanup pass on the code to try to spot any other such errors and make sure the Python code passes the flake8 linter.

            It would be even better to improve coverage of the unit tests beyond the minimum listed above. But I can see how that may have to be deferred.

            Show
            rowen Russell Owen added a comment - - edited I suggest the following as a minimum: Add tests that catch the reported bugs Fix the reported bugs Take a cleanup pass on the code to try to spot any other such errors and make sure the Python code passes the flake8 linter. It would be even better to improve coverage of the unit tests beyond the minimum listed above. But I can see how that may have to be deferred.
            Hide
            tjenness Tim Jenness added a comment -

            Have we converged on a plan?

            Show
            tjenness Tim Jenness added a comment - Have we converged on a plan?
            Hide
            krughoff Simon Krughoff added a comment -

            I think so. Implementation ticket is DM-5304, but that is blocking on several other tickets.

            Show
            krughoff Simon Krughoff added a comment - I think so. Implementation ticket is DM-5304 , but that is blocking on several other tickets.
            Hide
            yusra Yusra AlSayyad added a comment -

            John Parejko and Simon Krughoff is this ready to mark implemented?

            Show
            yusra Yusra AlSayyad added a comment - John Parejko and Simon Krughoff is this ready to mark implemented?

              People

              Assignee:
              krughoff Simon Krughoff
              Reporter:
              krughoff Simon Krughoff
              Watchers:
              Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Russell Owen, Simon Krughoff, Tim Jenness, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.