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

Remove measurement code from meas_algorithms

    Details

      Attachments

        Issue Links

          Activity

          Hide
          pgee Perry Gee added a comment -

          meas_base c369de
          meas_algorithms c76871
          pipe_tasks e0116

          These three were all rebased, so that might be the problem. The SHA1s are not in my histories, so you might need a new clone.

          Note that I just did another push -f on pipe_tasks as well.

          The others you listed match. I forgot about the meas_extensions_*, which were done during buildBot a couple of weeks ago. How did you find these (without looking through all the repos)?

          Show
          pgee Perry Gee added a comment - meas_base c369de meas_algorithms c76871 pipe_tasks e0116 These three were all rebased, so that might be the problem. The SHA1s are not in my histories, so you might need a new clone. Note that I just did another push -f on pipe_tasks as well. The others you listed match. I forgot about the meas_extensions_*, which were done during buildBot a couple of weeks ago. How did you find these (without looking through all the repos)?
          Hide
          pgee Perry Gee added a comment -

          Yes, the meas_extensions are part of the ticket. I made these changes on lsst-dev, and never pulled them to my working machine. This is one of the gotchas about git local repos. Useful, but potentially confusing if you don't have the habit of fetching constantly.

          Show
          pgee Perry Gee added a comment - Yes, the meas_extensions are part of the ticket. I made these changes on lsst-dev, and never pulled them to my working machine. This is one of the gotchas about git local repos. Useful, but potentially confusing if you don't have the habit of fetching constantly.
          Hide
          jbosch Jim Bosch added a comment - - edited

          Thanks for the clarifications. I'll make sure I get new versions of all of those.

          I found the meas_extensions_* branches by using my bot tool, but I also just noticed you can find them on this issue page, where it shows the git branches and pull requests associated with this issue.

          Show
          jbosch Jim Bosch added a comment - - edited Thanks for the clarifications. I'll make sure I get new versions of all of those. I found the meas_extensions_* branches by using my bot tool, but I also just noticed you can find them on this issue page, where it shows the git branches and pull requests associated with this issue.
          Hide
          jbosch Jim Bosch added a comment -

          Review complete. I've made some minor comments on PRs for meas_base and meas_algorithms, but I think all of those will be easy changes to make, so there's no need to check back with me before merging (unless you want to).

          Other packages look fine, so I didn't even make PRs for them (aside meas_astrom, where I just said that things were fine).

          However, I wonder if we can now remove the dependencies on meas_algorithms from the meas_extensions .table and .cfg files. Would you mind trying that and seeing if they still get through buildbot?

          Show
          jbosch Jim Bosch added a comment - Review complete. I've made some minor comments on PRs for meas_base and meas_algorithms, but I think all of those will be easy changes to make, so there's no need to check back with me before merging (unless you want to). Other packages look fine, so I didn't even make PRs for them (aside meas_astrom, where I just said that things were fine). However, I wonder if we can now remove the dependencies on meas_algorithms from the meas_extensions .table and .cfg files. Would you mind trying that and seeing if they still get through buildbot?
          Hide
          pgee Perry Gee added a comment -

          I swear that I made those debugging and cosmetic changes to measAlgs.py, PsfAttributes.cc, and ImagePsf.cc at least once before. The display=True for sure. Sorry I left that junk in.

          I am not able to remove the include lsst/meas/algorithms.h from hsmLib.i without some work, so I am going to skip that, and go ahead and run buildBot on the rest. It is probably something simple, but it is not obvious to me on cursory inspection. If you can spot it, send me email. I will probably wait until later the merge, when the buildBot is done.

          meas_extensions_photometryKron seems to be happy with the idea.

          The comment you made in ImagePsf about the separate declaration of the declaration of the const & from the _psfImage: I was getting a type error on the call to fitCentroid which had me convinced that I had to create a const & (or create in the call, which was pretty verbose looking).
          I am surprised to find that your suggestion compiles.

          Show
          pgee Perry Gee added a comment - I swear that I made those debugging and cosmetic changes to measAlgs.py, PsfAttributes.cc, and ImagePsf.cc at least once before. The display=True for sure. Sorry I left that junk in. I am not able to remove the include lsst/meas/algorithms.h from hsmLib.i without some work, so I am going to skip that, and go ahead and run buildBot on the rest. It is probably something simple, but it is not obvious to me on cursory inspection. If you can spot it, send me email. I will probably wait until later the merge, when the buildBot is done. meas_extensions_photometryKron seems to be happy with the idea. The comment you made in ImagePsf about the separate declaration of the declaration of the const & from the _psfImage: I was getting a type error on the call to fitCentroid which had me convinced that I had to create a const & (or create in the call, which was pretty verbose looking). I am surprised to find that your suggestion compiles.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel