scidash / neuronunit

A package for data-driven validation of neuron and ion channel models using SciUnit
http://neuronunit.scidash.org
38 stars 24 forks source link

NEURON backend models, should have an optional keyword DTC object argument in the constructor #138

Closed russelljjarvis closed 6 years ago

russelljjarvis commented 6 years ago

This might neaten up DTC-> neuronunit model update steps, and it could also provide a means to store current_source_name, and cell_name in DTCs and have them update models each time a new model object gets minted, with the advantage that a disk/file read could then be replaced with a memory read.

rgerkin commented 6 years ago

As it stands the backend= kwarg of the model doesn't just have to be a string like "NEURON", it can be a tuple (see here). In that tuple, the first item should be the name (e.g. "NEURON") and then subsequent items can be args (as a list) and kwargs as a dict.

So, for example:
backend = ("NEURON",[1,2,3],{'a':11,'b':22}) Will then invoke the init_backend for the NEURONBackend (see here with:
init_backend(1,2,3,a=11,b=22) rather than the vanilla init_backend() we usually call.

So all you have to do is set NEURON's init_backend to take an optional DTC kwarg, e.g. change this to something like def init_backend(self, DTC=None): and then you can check for DTC is not None and use it to set the various things provided by the DTC. Then all this should be called automatically when you instantiate the model with model = ReducedModel('/path/to/model', backend=('NEURON', {'DTC':dtc})) where dtc is some DTC instance.

russelljjarvis commented 6 years ago

I will need extra help to get this line model = ReducedModel('/path/to/model', backend=('NEURON', {'DTC':dtc})) working properly. Perhaps I can create a simple unittest for this.

rgerkin commented 6 years ago

If you write a simple unittest for this (which fails, presumably), I can take a look at it and see why it fails.

rgerkin commented 6 years ago

@russelljjarvis Does this work for you now?

rgerkin commented 6 years ago

Please make a formula reproducing this problem.

russelljjarvis commented 6 years ago

I think the problem is in the constructor. My reasons for thinking this come from eliminating other possibilities about what happens after the model constructor is called.

For example in models/backends/neuron.py specifically lines 16 - 53:

The condition:
if type(DTC) is not None: never executes. It's not about the syntax of this condition. For instance, using very explicit if type(DTC) is not type(None): also wouldn't work. Or if not isinstance(DTC,None) equally would not work.

I think the problem is instead, related to object construction, and the constructor method for models:

model = ReducedModel(get_neab.LEMS_MODEL_PATH,name=str('vanilla'),backend=('NEURON',{'DTC':dtc}))

Particularily in the way backend is assigned a tuple that contains a string as the first element, and a dictionary as the second element. I don't think this information is being handled properly in the constructor code, corresponding to this method signature.

If you scroll down in the snippet below, you can see how I have tried to test if particular code executes using print statements.

Generally set_attrs() is not called, and the model that is created based on the above constructor is very incomplete.

I can send through a recipea for recreating this bug based on a dedicated git branch.

    def init_backend(self, attrs = None, cell_name = None, current_src_name = None, DTC = None):
        '''Initialize the NEURON backend for neuronunit.
        Optional Key Word Arguments:
        Arguments: attrs a dictionary of items used to update NEURON model attributes.
        cell_name, and _current_src_name should not attain arbitrary values, rather these variable names
        may need to have consistency with an underlying jNEUROML model files:
        LEMS_2007One_nrn.py  LEMS_2007One.xml
        cell_name: A string that represents the cell models name in the NEURON HOC space.
        current_src_name: A string that represents the current source models name in the NEURON HOC space.
        DTC: An object of type Data Transport Container. The data transport container contains a dictionary of model attributes
        When the DTC object is provided, it\'s attribute dictionary can be used to update the NEURONBackends model attribute dictionary.
        '''
        if not NEURON_SUPPORT:
            raise BackendException("The neuron module was not successfully imported")

        self.neuron = None
        self.model_path = None
        self.h = h
        # Should check if a legitimately parallel MPI based NEURON is supported.
        self.lookup = {}

        super(NEURONBackend,self).init_backend()
        self.model.unpicklable += ['h','ns','_backend']
        if cell_name:
            self._cell_name = cell_name
        if current_src_name:
            self._current_src_name = current_src_name
        #if not isinstance(DTC, type(None)):
        if type(DTC) is not None:
            print('type \n\n\n\n\n\n', type(DTC), DTC)
            print(dir())

            assert 1==2
            print('never gets here \n\n\n\n\n\n')
            print(DTC.attrs, 'the dtc attrs empty?')
            self.model.set_attrs(DTC.attrs)
rgerkin commented 6 years ago

We agreed that this is solved.