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

Refactor meas_base Python wrappers and plugin registration

    XMLWordPrintable

Details

    Description

      meas_base currently has a single Swig library (like most packages), defined within a single .i file (like some packages). It also registers all of its plugins in a single python module, plugins.py.

      Instead, it should:

      • Have two Swig libraries: one for the interfaces and helper classes, and one for plugin algorithms. Most downstream packages will only want to %import (and hence #include) the interface, and having them build against everything slows the build down unnecessarily. The package _init_.py should import all symbols from both libraries, so the change would be transparent to the user.
      • Have separate .i files for each algorithm or small group of algorithms. Each of these could %import the interface library file and the pure-Python registry code, and then register the plugins wrapped there within a %pythoncode block. That'd make the implementation of the algorithms a bit less scattered throughout the package, making them easier to maintain and better examples for new plugins.

      Attachments

        Activity

          pgee Perry Gee added a comment -

          Jim:

          I was not able to find a solution to the SWIG problems I encountered when I tried to put the plugins into a separate swig library from the Results, ResultMappers, and Inputs. So as we talked about on Sunday, I put the plugins in separate .i files but included them in baseLib.i

          I also tried the experiment of creating a plugin in a separate module and importing baseLib.i. I found that this will only work if I include a line in the pythoncode in wrapMeasurementAlgorithmEx to do:

          from lsst.meas.base import *

          The reason is that if CONTROL or INPUT are defined in lsst.meas.base, they are not imported by the outside module swig library and are not found when the pythoncode is executed.

          I would like a better solution to this problem, if you have one.

          pgee Perry Gee added a comment - Jim: I was not able to find a solution to the SWIG problems I encountered when I tried to put the plugins into a separate swig library from the Results, ResultMappers, and Inputs. So as we talked about on Sunday, I put the plugins in separate .i files but included them in baseLib.i I also tried the experiment of creating a plugin in a separate module and importing baseLib.i. I found that this will only work if I include a line in the pythoncode in wrapMeasurementAlgorithmEx to do: from lsst.meas.base import * The reason is that if CONTROL or INPUT are defined in lsst.meas.base, they are not imported by the outside module swig library and are not found when the pythoncode is executed. I would like a better solution to this problem, if you have one.
          jbosch Jim Bosch added a comment -

          At this point, I don't think we should worry about it. I think that Swig macro will go away with virtually any sensible C++ redesign, and I think we'll be doing that at the beginning of the next sprint, if not sooner.

          jbosch Jim Bosch added a comment - At this point, I don't think we should worry about it. I think that Swig macro will go away with virtually any sensible C++ redesign, and I think we'll be doing that at the beginning of the next sprint, if not sooner.
          pgee Perry Gee added a comment -

          The scope of this ticket was cut down after we failed to find a way to make the wrapper macros in baseLib.i work against a second SWIG dynamic library in the same directory. The technique of importing baseLib.i works from outside lsst/meas/base, but not for a second dynamic library in the same module.

          So the only change is to move all the algorithm related stuff into pluginsLib.i, and include that in baseLib.i

          Code is in meas_base, u/pgee/DM-1217. baseLib.i was changed to include pluginsLib.i

          pgee Perry Gee added a comment - The scope of this ticket was cut down after we failed to find a way to make the wrapper macros in baseLib.i work against a second SWIG dynamic library in the same directory. The technique of importing baseLib.i works from outside lsst/meas/base, but not for a second dynamic library in the same module. So the only change is to move all the algorithm related stuff into pluginsLib.i, and include that in baseLib.i Code is in meas_base, u/pgee/ DM-1217 . baseLib.i was changed to include pluginsLib.i
          jbosch Jim Bosch added a comment -

          One quibble: if we're not building pluginsLib.i on its own, we should rename it to just "plugins.i", as I think the "Lib" suffix tends to imply that it's a separate Swig build.

          jbosch Jim Bosch added a comment - One quibble: if we're not building pluginsLib.i on its own, we should rename it to just "plugins.i", as I think the "Lib" suffix tends to imply that it's a separate Swig build.
          pgee Perry Gee added a comment -

          True, though it did once have its own dll. I will change the name.

          A couple of comments. As I told you, I did test that I could build a completely separate library an import baseLib.i with it. It works when the library is outside of lsst.meas.base, but not if it is inside. That's the SWIG issue you helped me detect. However, just for the record, this external Lib requires a slightly different pythoncode, with an import.meas.base in it.

          The other thing is not I did not move the plugin registration out of plugins.py, as it cannot be done in pythoncode when the plugin is in lsst.meas.base (due to the fact that the python wrapper class is not available at that point). And for the meas_base plugins, plugins.py seems like as good a place as any to create the wrapper. We can revisit this after the C++ redesign if this isn't what you want.

          It does work when the plugin is in an external package to create the wrapper in pythoncode.

          pgee Perry Gee added a comment - True, though it did once have its own dll. I will change the name. A couple of comments. As I told you, I did test that I could build a completely separate library an import baseLib.i with it. It works when the library is outside of lsst.meas.base, but not if it is inside. That's the SWIG issue you helped me detect. However, just for the record, this external Lib requires a slightly different pythoncode, with an import.meas.base in it. The other thing is not I did not move the plugin registration out of plugins.py, as it cannot be done in pythoncode when the plugin is in lsst.meas.base (due to the fact that the python wrapper class is not available at that point). And for the meas_base plugins, plugins.py seems like as good a place as any to create the wrapper. We can revisit this after the C++ redesign if this isn't what you want. It does work when the plugin is in an external package to create the wrapper in pythoncode.
          rowen Russell Owen added a comment -

          This looks like a nice, simple cleanup.

          rowen Russell Owen added a comment - This looks like a nice, simple cleanup.
          jbosch Jim Bosch added a comment -

          Perry, could you confirm that this has been merged to master, and close the issue if that's the case?

          jbosch Jim Bosch added a comment - Perry, could you confirm that this has been merged to master, and close the issue if that's the case?

          People

            pgee Perry Gee
            jbosch Jim Bosch
            Russell Owen
            Jim Bosch, Perry Gee, Russell Owen
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Jenkins

                No builds found.