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

Resolve compiler warnings in new measurement framework

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_base
    • Labels:
      None

      Description

      When building meas_base, or any other measurement plugins which follow the same interface, with clang, I see a bunch of warnings along the lines of:

      In file included from src/ApertureFlux.cc:34:
      include/lsst/meas/base/ApertureFlux.h:197:18: warning: 'lsst::meas::base::ApertureFluxAlgorithm::measure' hides overloaded virtual function
            [-Woverloaded-virtual]
          virtual void measure(
                       ^
      include/lsst/meas/base/Algorithm.h:183:18: note: hidden overloaded virtual function 'lsst::meas::base::SimpleAlgorithm::measure' declared here:
            different number of parameters (4 vs 2)
          virtual void measure(

      This is an artefact of a workaround for SWIG issues; the warnings aren't indicative of a fundamental problem, but if we can avoid them we should.

      While we're at it, we should also fix:

      include/lsst/meas/base/ApertureFlux.h:233:1: warning: 'ApertureFluxResult' defined as a struct here but previously declared as a class
            [-Wmismatched-tags]
      struct ApertureFluxResult : public FluxResult {
      ^
      include/lsst/meas/base/ApertureFlux.h:65:1: note: did you mean struct here?
      class ApertureFluxResult;
      ^~~~~
      struct

        Attachments

          Issue Links

            Activity

            Hide
            lauren Lauren MacArthur added a comment -

            I am indeed happy with all of the above. Those particular warnings are now gone and all tests passed, so feel free to make and merge the other related changes when you can without further review.

            Show
            lauren Lauren MacArthur added a comment - I am indeed happy with all of the above. Those particular warnings are now gone and all tests passed, so feel free to make and merge the other related changes when you can without further review.
            Hide
            swinbank John Swinbank added a comment -

            Jim – following our discussion, I tweaked meas_base to disambiguate measure and measureForced. Changes on tickets/DM-2131 in meas_base. Passes the tests there, and also resolves my issues in ip_diffim.

            I realise you are snowed under with end-of-cycle reviews, but could you give it a quick check to make sure it's what you had in mind?

            Also, since this is a change to the previously-agreed interface, any thoughts on whether it needs RFC-ing? My feeling is no.

            Show
            swinbank John Swinbank added a comment - Jim – following our discussion, I tweaked meas_base to disambiguate measure and measureForced . Changes on tickets/DM-2131 in meas_base . Passes the tests there, and also resolves my issues in ip_diffim . I realise you are snowed under with end-of-cycle reviews, but could you give it a quick check to make sure it's what you had in mind? Also, since this is a change to the previously-agreed interface, any thoughts on whether it needs RFC-ing? My feeling is no.
            Hide
            jbosch Jim Bosch added a comment -

            Looks good; sorry it took me so long to take a look at this. I agree that this shouldn't require an RFC.

            Show
            jbosch Jim Bosch added a comment - Looks good; sorry it took me so long to take a look at this. I agree that this shouldn't require an RFC.
            Hide
            swinbank John Swinbank added a comment -

            Thanks! ("So long" being two weekend days? Hmm...!)

            Show
            swinbank John Swinbank added a comment - Thanks! ("So long" being two weekend days? Hmm...!)
            Hide
            swinbank John Swinbank added a comment -

            I'd like to land this now, but it turns out I need to install GalSim before I can test on shapeHSM, and I doubt that's going to happen before I have to bump this to S15.

            Show
            swinbank John Swinbank added a comment - I'd like to land this now, but it turns out I need to install GalSim before I can test on shapeHSM , and I doubt that's going to happen before I have to bump this to S15.

              People

              • Assignee:
                swinbank John Swinbank
                Reporter:
                swinbank John Swinbank
                Reviewers:
                Jim Bosch
                Watchers:
                Jim Bosch, John Swinbank, Lauren MacArthur, Perry Gee
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel