Problem in adding multiple Python modules to ModuleManager

[keceli]

I have been observing intermittent errors with Python modules like IndexError: map::at with mod.change_input() when I have more than one Python module in the module manager. The problem never happens if I have only one Python module, but happens randomly if I have more than one. I think the code below shows the root of the problem.

import pluginplay as pp
from py_test_pluginplay import OneInOneOut, OneIn, OneOut

ptype = OneInOneOut()
class test_module(pp.ModuleBase):
    """Basic PluginPlay module that satisfies OneInOneOut property type"""
    def __init__(self):
        pp.ModuleBase.__init__(self)
        self.description(self.__doc__)
        self.satisfies_property_type(ptype)

    def run_(self, inputs, submods):
        s, = ptype.unwrap_inputs(inputs)
        r = self.results()
        return ptype.wrap_results(r, s)

ptype = OneIn()
class test_module2(pp.ModuleBase):
    """Basic PluginPlay module that satisfies OneIn property type"""
    def __init__(self):
        pp.ModuleBase.__init__(self)
        self.description(self.__doc__)
        self.satisfies_property_type(ptype)

    def run_(self, inputs, submods):
        s, = ptype.unwrap_inputs(inputs)
        r = self.results()
        return ptype.wrap_results(r, s)
ptype = OneOut()
class test_module3(pp.ModuleBase):
    def __init__(self):
        pp.ModuleBase.__init__(self)
        self.satisfies_property_type(ptype)
        self.add_input('inp2')

    def run_(self, inputs, submods):
        r = self.results()
        return ptype.wrap_results(r)

mm = pp.ModuleManager()
mm.add_module('test_module', test_module())
mm.add_module('test_module2', test_module2())
mm.add_module('test_module3', test_module3())
for key in mm.keys():
    print(key, mm[key])
print(mm['test_module'] == mm['test_module2'])
print(mm.at('test_module') == mm.at('test_module2'))

If you run the Python code above several times, you will get something like:

(nwx) 01:15:00|nwx|test> python mm_test3.py
test_module <pluginplay.Module object at 0x7fafa0d2a330>
test_module2 <pluginplay.Module object at 0x7fafa0d24a70>
test_module3 <pluginplay.Module object at 0x7fafa0d1d970>
True
True
(nwx) 01:15:02|nwx|test> python mm_test3.py
test_module <pluginplay.Module object at 0x7f290c631530>
test_module2 <pluginplay.Module object at 0x7f290c631530>
test_module3 <pluginplay.Module object at 0x7f290c631530>
True
True

Somehow in the second run, test_modules that satisfy different property types are stored at the same memory location. Module comparison always returns True might be because the comparison operator not being exported, but I think main issue is that Python modules are not added to the module manager properly. Maybe this issue is related to #309 somehow.

[jwaldrop107]

When python Modules are added to the ModuleManager, they all seem to get assigned the same type index at this line. Following export_module_manager.cpp to py_module_base.hpp and back to module_base.hpp, the type that the index corresponds to is python::PythonWrapper. So even though a new Module is made and added to the manager's list of Modules, all of the python modules have the same underlying ModuleBase. The fix for this is ensuring that the types of the python Modules are distinct, but I don't know for sure how to accomplish that. @ryanmrichard, any ideas?

[ryanmrichard]

Long story short, rather than relying on type info for versioning modules we should do this better. Right now PluginPlay assumes that if two modules have the same type they contain the same version of the same implementation. In practice, this is not a sufficient check. What should probably happen is that we introduce a new class say ModuleVersion which can be used to compare module instances. The C++ version can just store the std::type_info. The Python version would use reflection to work out a string representation of the derived type. You could also throw a version string in there too for good measure (IIRC there's already a metadata attribute for this).

If you're going to tackle this now I'd probably quickly sketch out a design doc before you do it. In particular you'll need to work out how the Python vs. C++ versions will work (off the top of my head, I'd probably go polymorphic PIMPL) and also try to foresee what other versioning info we may need.

[jwaldrop107]

Yeah, I don't plan on addressing this myself right now. Just wanted to put what I'd found out there. If someone else wants to tackle it, cool.