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

Remove or revive bitrotted code in meas_modelfit

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_modelfit
    • Labels:
      None
    • Story Points:
      2
    • Epic Link:
    • Sprint:
      DRP S17-3
    • Team:
      Data Release Production

      Description

      meas_modelfit contains a lot of code that was used for FDR-era prototyping of modelfitting and has not been used since. Some of this is worth keeping and reviving, because it's hard-won algorithmic code we may want to use in the future, while some of it should be removed. In many cases, the unwanted code is being kept around because it's the only way the code we want is still tested; in other cases a small, easy-to-replace fraction of it is used by the bitrotted code we want to keep or code in active use (i.e. CModel).

      This ticket is for cleaning up that mess. Roughly, that means:

      • Remove the custom table/record/catalog classes.
      • Remove the custom CmdLineTasks.
      • Remove the Sampler and Interpreter classes.

      Some of this work may be best spawned done on other tickets, which should be linked here.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            I've spun off the harder tasks in this issue into a new one, in order to make this one doable quickly. I think doing that before converting meas_modelfit to pybind11 will save time overall, as this issue will greatly reduce the amount of Python code that needs to be wrapped.

            I've adjusted the story points of this issue accordingly.

            Show
            jbosch Jim Bosch added a comment - I've spun off the harder tasks in this issue into a new one, in order to make this one doable quickly. I think doing that before converting meas_modelfit to pybind11 will save time overall, as this issue will greatly reduce the amount of Python code that needs to be wrapped. I've adjusted the story points of this issue accordingly.
            Hide
            jbosch Jim Bosch added a comment -

            Nate Lust, could you take a look at this deletion-only (and therefore hopefully trivial) review?

            If you're curious, this is all the stuff you now will not have to deal with when you start to take ownership of meas_modelfit

            Show
            jbosch Jim Bosch added a comment - Nate Lust , could you take a look at this deletion-only (and therefore hopefully trivial) review? If you're curious, this is all the stuff you now will not have to deal with when you start to take ownership of meas_modelfit
            Hide
            nlust Nate Lust added a comment -

            I have spent a while reading over this. I see no obvious issues, but it is pretty dense. If it passes building and tests (do we have a ci-hsc test that looks at M.M.F.?, if so please run that in addition to jenkins) then I think this is fine to merge.

            Show
            nlust Nate Lust added a comment - I have spent a while reading over this. I see no obvious issues, but it is pretty dense. If it passes building and tests (do we have a ci-hsc test that looks at M.M.F.?, if so please run that in addition to jenkins) then I think this is fine to merge.
            Hide
            jbosch Jim Bosch added a comment -

            Thanks. Running ci_hsc is a good test that I hadn't thought to do myself (meas_modelfit does provide CModel and the PSF-approximation algorithms it uses, and while I think I've been pretty careful not to delete anything those use, another test is always a good idea).

            Show
            jbosch Jim Bosch added a comment - Thanks. Running ci_hsc is a good test that I hadn't thought to do myself (meas_modelfit does provide CModel and the PSF-approximation algorithms it uses, and while I think I've been pretty careful not to delete anything those use, another test is always a good idea).
            Hide
            jbosch Jim Bosch added a comment -

            Merged to master after successful ci_hsc run (not on OS X, but I think that's unrelated; see DM-9576).

            Show
            jbosch Jim Bosch added a comment - Merged to master after successful ci_hsc run (not on OS X, but I think that's unrelated; see DM-9576 ).

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Nate Lust
              Watchers:
              Jim Bosch, Nate Lust
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.