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

remove old forced photometry tasks

    XMLWordPrintable

    Details

      Description

      After meas_base has been fully integrated, remove the old forced photometry tasks from pipe_tasks

        Attachments

          Issue Links

            Activity

            jbosch Jim Bosch created issue -
            jbosch Jim Bosch made changes -
            Field Original Value New Value
            Epic Link DM-16 [ 11264 ]
            jbosch Jim Bosch made changes -
            Rank Ranked higher
            jbosch Jim Bosch made changes -
            Rank Ranked lower
            jbosch Jim Bosch made changes -
            Rank Ranked higher
            jbosch Jim Bosch made changes -
            Rank Ranked higher
            jbosch Jim Bosch made changes -
            Epic Link DM-16 [ 11264 ] DM-1101 [ 13908 ]
            jbosch Jim Bosch made changes -
            Labels Measurement AppsPlanning
            jbosch Jim Bosch made changes -
            Labels AppsPlanning SciencePipelines
            jbosch Jim Bosch made changes -
            Team Measurement Framework [ 10205 ] Princeton [ 10301 ]
            jbosch Jim Bosch made changes -
            Sprint Science Pipelines DM-W15-5 [ 129 ]
            pgee Perry Gee made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            swinbank John Swinbank made changes -
            Epic Link DM-1101 [ 13908 ] DM-1769 [ 15588 ]
            swinbank John Swinbank made changes -
            Sprint Science Pipelines DM-W15-5 [ 129 ] Science Pipelines DM-W15-5, Science Pipelines DM-S15-1 [ 129, 140 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            jbosch Jim Bosch made changes -
            Link This issue blocks DM-420 [ DM-420 ]
            Hide
            pgee Perry Gee added a comment -

            This seemed like a very simple 1 story point, but perhaps I missed something. Or perhaps it is one SP, rounded up.

            pgeepc2:/sandbox2/pgee/mylsst9/Linux64/pipe_tasks> git diff --stat master
            bin/forcedPhot.py | 4 -
            python/lsst/pipe/tasks/forcedPhot.py | 202 ----------------------------------
            2 files changed, 206 deletions

            Show
            pgee Perry Gee added a comment - This seemed like a very simple 1 story point, but perhaps I missed something. Or perhaps it is one SP, rounded up. pgeepc2:/sandbox2/pgee/mylsst9/Linux64/pipe_tasks> git diff --stat master bin/forcedPhot.py | 4 - python/lsst/pipe/tasks/forcedPhot.py | 202 ---------------------------------- 2 files changed, 206 deletions
            pgee Perry Gee made changes -
            Reviewers John Swinbank [ swinbank ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            jbosch Jim Bosch added a comment -

            There are also the old references tasks, I think, and maybe something in obs_sdss.

            Show
            jbosch Jim Bosch added a comment - There are also the old references tasks, I think, and maybe something in obs_sdss.
            Hide
            pgee Perry Gee added a comment -

            I ran it through buildBot, which is my way of finding actual dependencies. But of course, it doesn't find dead code. Thanks.

            Show
            pgee Perry Gee added a comment - I ran it through buildBot, which is my way of finding actual dependencies. But of course, it doesn't find dead code. Thanks.
            Hide
            pgee Perry Gee added a comment -

            I've added a u/pgee/DM-1073 to obs_sdss to remove the forcedPhot.py there. I don't see any references task remaining in pipe_tasks.

            Show
            pgee Perry Gee added a comment - I've added a u/pgee/ DM-1073 to obs_sdss to remove the forcedPhot.py there. I don't see any references task remaining in pipe_tasks.
            Hide
            swinbank John Swinbank added a comment -

            It doesn't look as though you've actually pushed your branch to the pipe_tasks repository. Could you check?

            Show
            swinbank John Swinbank added a comment - It doesn't look as though you've actually pushed your branch to the pipe_tasks repository. Could you check?
            Hide
            swinbank John Swinbank added a comment - - edited

            Other than the fact that I can't actually see your branch, I think I'm broadly happy with this; since you're just removing code, there's nothing for me to to do except check that it's really gone!

            The references tasks Jim refers to in pipe_tasks are included in the forcedPhot.py file, so I think you've got them. However, in obs_sdss, note the following places you've missed:

            • config/forcedPhot.py (I'm actually not sure if this config needs updating to match the new tasks in meas_base or if it can just be dropped, but it's certainly not correct as is);
            • policy/SdssMapper.paf line 406 refers to lsst.pipe.tasks.forcedPhot.ForcedPhotConfig.

            I also note that bin/ingest/ingestForcedSource.py in the datarel repository refers to both lsst.obs.sdss.forcedPhot and lsst.obs.lsstSim.forcedPhot. This code is pretty old, and I don't know if it's still relevant; you should find out before you remove that code from obs_sdss.

            Finally – and this is not relevant for this issue, and may be more a question for Jim Bosch – why do processImage.py, processCcd.py etc live in pipe_tasks, but forcedPhotImage.py, forcedPhotCcd.py etc live in meas_base?

            Show
            swinbank John Swinbank added a comment - - edited Other than the fact that I can't actually see your branch, I think I'm broadly happy with this; since you're just removing code, there's nothing for me to to do except check that it's really gone! The references tasks Jim refers to in pipe_tasks are included in the forcedPhot.py file, so I think you've got them. However, in obs_sdss , note the following places you've missed: config/forcedPhot.py (I'm actually not sure if this config needs updating to match the new tasks in meas_base or if it can just be dropped, but it's certainly not correct as is); policy/SdssMapper.paf line 406 refers to lsst.pipe.tasks.forcedPhot.ForcedPhotConfig . I also note that bin/ingest/ingestForcedSource.py in the datarel repository refers to both lsst.obs.sdss.forcedPhot and lsst.obs.lsstSim.forcedPhot . This code is pretty old, and I don't know if it's still relevant; you should find out before you remove that code from obs_sdss . Finally – and this is not relevant for this issue, and may be more a question for Jim Bosch – why do processImage.py , processCcd.py etc live in pipe_tasks , but forcedPhotImage.py , forcedPhotCcd.py etc live in meas_base ?
            swinbank John Swinbank made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            Hide
            jbosch Jim Bosch added a comment -

            Finally – and this is not relevant for this issue, and may be more a question for Jim Bosch – why do processImage.py, processCcd.py etc live in pipe_tasks, but forcedPhotImage.py, forcedPhotCcd.py etc live in meas_base?

            I moved the forced photometry tasks to meas_base because they're closely tied to lower-level code in meas_base - and I was at that point frustrated that we hadn't done a better job of considering where to put some of the code that's currently in pipe_tasks. This will all get cleaned up further in DM-2003.

            Show
            jbosch Jim Bosch added a comment - Finally – and this is not relevant for this issue, and may be more a question for Jim Bosch – why do processImage.py , processCcd.py etc live in pipe_tasks, but forcedPhotImage.py , forcedPhotCcd.py etc live in meas_base? I moved the forced photometry tasks to meas_base because they're closely tied to lower-level code in meas_base - and I was at that point frustrated that we hadn't done a better job of considering where to put some of the code that's currently in pipe_tasks. This will all get cleaned up further in DM-2003 .
            Hide
            pgee Perry Gee added a comment -

            I want to know what you used to find these references. Do you have a code tree search tool, or are you just more persistent than I was? Thanks.

            Show
            pgee Perry Gee added a comment - I want to know what you used to find these references. Do you have a code tree search tool, or are you just more persistent than I was? Thanks.
            Show
            swinbank John Swinbank added a comment - GitHub – https://github.com/search?q=forcedPhot+user%3Alsst&type=Code&utf8= ✓
            Hide
            pgee Perry Gee added a comment -

            Cool. It's no excuse. I could have used grep. But it is nice to have something smarter.

            Show
            pgee Perry Gee added a comment - Cool. It's no excuse. I could have used grep. But it is nice to have something smarter.
            pgee Perry Gee made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]

              People

              Assignee:
              pgee Perry Gee
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              John Swinbank
              Watchers:
              Jim Bosch, John Swinbank, Perry Gee
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: