Increasing modularity of the new submodel system

[jlstevens]

The new submodel system has recently been merged (#571).

It looks like this system will be a substantial improvement over our existing ty file format but there are still some features that need polishing. I am opening this issue to have one place where we can discuss new improvements and changes.

EDIT: This thread started as a general discussion about improvements but gradually got hijacked by a discussion about making the system more modular. The is the new wiki page to discuss miscellaneous improvements.

Here are the Issues that have been spun out of the original discussions:

Tab completion when defining submodel parameters: #574

[jlstevens]

I have just made a bunch of commits working together with Philipp which should clarify how everything works together. All in all, I really like it!

There are only two major annoyances I have with the system right now:

• Need to specify parameters as strings instead of using some convenient tab-completion.
• The need for the modeler to create SheetSpec objects in setup_sheets. I would like to hide as much machinery from the modeler as possible and if we can specify our models with simple Python literals (that anybody can understand), we should!

Right now, I only have vague ideas how to solve the first problem so I will discuss my proposal for the second issue.

Why can't setup_sheets return a dictionary-of-properties instead of a list of SheetSpec objects? For instance, part of the return value (just for the LGN level) could look something like:

{'LGN':[{'polarity':'ON'}, {'polarity':'OFF'}]}

Here the 'level' is the key and the list elements are the properties of the sheets to be defined at that level. Of course you would dynamically need to generate this dictionary as the dimensions change (to adapt the number of sheets in the model) but you need to do that anyway.

The only information that is now missing is the sheet type which is currently supplied to instantiate the SheetSpecs objects. I feel it would be cleaner to supply this information in the level decorator. Then we would use the decorators like this:

@Model.level('V1', CFSheet)
def V1_level_parameters(self, properties):
     return {'tsettle':16,
             'plastic':True,
             ...
             'nominal_density':self.cortex_density}

This makes more sense to me but this would also impose the restriction that the same type of sheet must be used across each level. Nothing stops you defining more levels if you need to change the sheet type of course!

What do you think? Is this neater or maybe you object to forcing all sheets on one level to share a sheet type?

[Tobias-Fischer]

Regarding the second issue: Yes, this looks much cleaner than the current implementation. For me, it makes totally sense that all sheets on one level share one sheet type, as you set parameters per level. It would be weird to set the same set of parameters to different sheet types. I am happy to implement the changes as soon as Jim gives his okay.

[Tobias-Fischer]

The current implementation does not work with SheetSpec properties which have non-string values, as name+=prop fails (name is a string). This is currently the case for spatial frequency sheets.

There are two ways to fix this:

1. Properties must always have string values, which would need to be documented. The drawback with this fix is that things like channel=properties['SF'] if 'SF' in properties else 1 would become more slightly more complicated, i.e. channel=int(properties['SF'][2:]) if 'SF' in properties else 1 However, it would have the advantage that we could stick to the current naming convention.
2. The simplest solution would be changing name+=prop to name+=str(prop). However, then SF sheets would be called LGNOn1, LGNOn2 and so forth. As long as there is only SF, this is fine. However additional "integer-"properties would make it harder to distinguish which sheet is meant, at least from the sheet name. Internally, everything would be still nice because the properties can always be accessed.

[philippjfr]

Having gone through the code with Jean-Luc today, I can only say you did a great job and the system is definitely a huge improvement over ty files.

I was on the fence about enforcing a particular sheet type for a sheet level, as I often create helper sheets, e.g. to visualize the subthreshold responses, but you convinced me, they should be a different level. So I definitely support making those changes.

I'll create some model subclasses for my model and report back on the experience.

[jbednar]

I'm happy with having the type specified along with the level, as indeed it makes sense to require a different level for a different type. And I agree with Philipp that this is a vast improvement; thanks for doing such a great job!

For non-string properties, I can start to appreciate the issue, but can't get my head around all of the issues enough to suggest a good solution. I do think that we'll run into trouble if integer-valued properties are all concatenated together unreadably. Maybe check the type, and if it's not string, prepend the string name of the property before concatenating with the property value? E.g. for property SF with value 1, yield SF1, for property center_size with value 0.5 yield center_size0.5, but for property color="red" yield red? Not sure if this makes sense; just trying to see if there is a consistent interpretation of our current conventions...

[jbednar]

While we're making notes about submodels, this is totally not something to try to address now, but in the long run I'm wondering whether it will be maintainable to have a class-based system for adding new levels. E.g. I'd originally thought that submodels would have objects that a .ty file could mix and match, so that a GCALCorticalLayer object could be instantiated once as V1, specifying that it matches connections from the LGN, and then instantiated again as V2, specifying that it matches connections from V1. Right now, I think we'd need a new class extending GCALModel, but I think it's not crazy to think we could be reusing cortical area models by instantiation alone.

[jlstevens]

I'm happy to implement the new system I suggested. :-)

I'll need to read what Tobias wrote about non-string properties again before I understand the issue properly.

[Tobias-Fischer]

Thanks Jean-Luc!

[Tobias-Fischer]

Jim, I think using the decorators it is quite easy to model what you try to achieve.

@Model.level('V1')
def _V1_level_parameters(self, properties):
    ...

@Model.matchconditions('V1')
def V1_matchconditions(self, properties):
    ....

@Model.connection('AfferentV1On',  projection.CFProjection)
@Model.connection('AfferentV1Off', projection.CFProjection)
def V1_afferent_projections(self, proj):
    ...

@Model.connection('LateralV1Excitatory', projection.CFProjection)
def V1_lateralexcitatory_projections(self, proj):
    ...

@Model.connection('LateralV1Inhibitory', projection.CFProjection)
def V1_lateralinhibitory_projections(self, proj):
    ...

Becomes

@Model.level('V1')
@Model.level('V2')
def _GCAL_level_parameters(self, properties):
    ...

@Model.matchconditions('V1')
def V1_matchconditions(self, properties):
    ....

@Model.matchconditions('V2')
def V1_matchconditions(self, properties):
    return {'AfferentV2': {'level': 'V1'}}

@Model.connection('AfferentV1On',  projection.CFProjection)
@Model.connection('AfferentV1Off', projection.CFProjection)
def V1_afferent_projections(self, proj):
    ...

@Model.connection('AfferentV2',  projection.CFProjection)
def V2_afferent_projections(self, proj):
    # Needs to be implemented, as no projection between lateral sheets has been existing so far

@Model.connection('LateralV1Excitatory', projection.CFProjection)
@Model.connection('LateralV2Excitatory', projection.CFProjection)
def GCAL_lateralexcitatory_projections(self, proj):
    ...

@Model.connection('LateralV1Inhibitory', projection.CFProjection)
@Model.connection('LateralV2Inhibitory', projection.CFProjection)
def GCAL_lateralinhibitory_projections(self, proj):
    ...

So all what needs to be done is adding a couple of decorators and implementing one new method. I agree that it would be even nicer to have objects which can be mix and matched, and will think of that soon. The above is showing that it is straightforward in the current system already though :)

[jbednar]

Thanks; sounds reasonable. It still requires changing topo.submodels, while I'm thinking of this as being an instantiation-time choice, but it doesn't look too bad. In any case, not something for you to spend time on, now that it covers the models we envisioned for your project.

[jlstevens]

I've had a thought which may be somewhat related...

Say ModelGCAL specifies a load of parameters for a projection. Here is a real example:

    @Model.connection('LateralV1Excitatory', projection.CFProjection)
    def V1_lateralexcitatory_projections(self, proj):
        return {'delay':0.05,
                'name':'LateralExcitatory',
                'weights_generator': Gaussian(aspect_ratio=1.0, size=0.05),
                'strength':self.exc_strength,
                'learning_rate':self.exc_lr,
                'nominal_bounds_template':sheet.BoundingBox(
                                                radius=self.latexc_radius)}

Now I want to implement ModelTCAL which for some reason needs to change only one little parameter (for instance, say I wish to double the excitatory strength). I think I could use the following definition in the ModelTCAL class:

    @Model.connection('LateralV1Excitatory', projection.CFProjection)
    def V1_lateralexcitatory_projections(self, proj):
       params = super(ModelTCAL, self).V1_lateralexcitatory_projections(proj)
       params[strength] *= 2
       return params

The reason I think this should work is that even though the key 'LateralV1Excitatory' is registered in the connection decorator in the ModelGCAL class as the new definition will take its place. This is simply because the decorator keeps track of the decorated methods internally using a dictionary and the keys must be unique in a dictionary.

This approach is extensible because I only specify what is different from the GCAL superclass without having to duplicate any parameters unnecessarily.

Maybe I am being stupid and this was the plan all along but I don't think I've seen an example of this used in the code we have so far...

[jbednar]

Sounds elegant to me...

[Tobias-Fischer]

That's done in ColorEarlyVision already:

@Model.connection('Afferent', projection.SharedWeightCFProjection)
def LGN_afferent_projections(self, proj):
    parameters = super(ColorEarlyVisionModel,self).LGN_afferent_projections(proj)
    if 'opponent' in proj.dest.properties:
        parameters['name']+=proj.dest.properties['opponent']+proj.src.properties['cone']
    return parameters

[jlstevens]

Tobias: Thanks! I must have seen it but not thought properly about what it meant! :-)

I see what you mean about instantiating objects instead of using classes. As we've discussed, at least we have the option of doing the following in the ty file without needing to define a new class:

gcal = ModelGCAL()
gcal.projections.V1.LGNOn.LGNOnAfferent.parameters['strength'] *= 2
gcal()

This isn't quite what you asked for but this is as close as I think we can get with this system right now...

Note: I'm partially writing this to clarify to myself how this system is to be used (and maybe as documentation?)

[Tobias-Fischer]

Yes, I use something similar all the time, e.g. to experiment with the learning rate.

[jbednar]

That looks good, though I'd have a tough time figuring out the 7-level atttribute hierarchy there. IPython tab-complete, I guess?

In any case, if it were truly being done "all the time", then presumably a Model-level parameter can be added to make changing that parameter easier.

Plus, adding Model-level parameters now has much lower cost -- they used to be in every .ty file, and so we had to rigorously trim any that were not guaranteed useful, but now it's just adding one parameter in one place in topo.submodels. Plus each such parameter normally comes with documentation, making adding it a benefit to the reader rather than weighing them down. So in the long run I expect to see more Model-level parameters for things like learning rates, if we often need to change them. That said, I don't recommend messing with the learning rate for normal usage of our models; much better to make sure that activity values are in the right range instead!

[jlstevens]

That looks good, though I'd have a tough time figuring out the 7-level attribute hierarchy there. IPython tab-complete, I guess?

Right. That is exactly how I generated that example. At least I can see a good reason for every one of those seven levels (unless we were happy for force unique names for projections).

In any case, if it were truly being done "all the time", then presumably a Model-level parameter can be added to make changing that parameter easier.

Agreed. There is always the decision whether a value is important enough to become a model parameter.

I also agree that adding new parameters now comes at a lower cost. That said, I wouldn't want to see the number of class parameters could explode either!

[jbednar]

Right. I wouldn't want to enforce globally unique projection names, but note that incoming projections are guaranteed to have unique names for a given target sheet, so in principle gcal.projections.V1.LGNOn.LGNOnAfferent could be done as gcal.projections.V1.LGNOnAfferent. In practice there may be some other reason to have LGNOn explicitly, which is fine if so.

BTW, I'm surprised to see you happy with .parameters['strength'] instead of .parameters.strength. :-)

[jlstevens]

I'm feeling quite torn about this..

On one hand I can't stand using strings when I can use tab-completion...on the other, AttrDicts are a scourge (and one that is still to be completely eliminated.).

Now you've pointed this out, I will have to find a way to fix this which means I now have two tab-completion problems to think about!

Before I forget, are we happy with the @Model qualification for all the decorators? I think it would be reasonable to first do from submodels import level, connection, matchconditions and then use those directly. What do you think?

I also think that we should remove that one unnecessary attribute level if we can. Tobias?

[jbednar]

I don't have any opinion about whether to specify Model explicitly for the decorators; it's verbose but also gives a clear ordering to everything, so there are arguments both ways.

[jbednar]

What will we do for GCAL models that don't use this particular LGN model? For instance, with Yvonne's random-wiring work, the complicated set of RGC/LGN sheets can be replaced with a single sheet or pair of sheets with random RGC wiring. The GCAL V1 model should be exactly the same as in the code already implemented, including having connections from all sheets witih "LGN" in their name, but because the subcortical pathway differs, I don't see how it would be implemented without duplicating the GCALModel class with no changes but a different name and parent class. Similarly, GCAL used for an auditory cortex model might be exactly the same except for the name of its input sheets; would we again have to duplicate the class? Can we somehow separate GCAL from the subcortical pathway, allowing GCAL to be attached to anything we like? Having the subcortical pathways tied together through inheritance doesn't seem limiting, but having GCALModel (and LISSOMModel, etc.) similarly tied does.

[jlstevens]

Good point.

GCAL is not a V1 level concept - instead it is a 'GC' concept for gain control at the LGN level and an 'AL' concept at the V1 level.

I don't think gain-control alone is sufficient to warrant having to combine a GC class and an AL class (because then toggling between the L,AL and GCL classes would be trickier to toggle between via script parameters).

Instead, I think it might be fair to move a gain control switch and definition to the EarlyVisionModel with a default of False which the GCAL model then sets to True. Contrast-gain control (correct hyphenation?) seems general enough to me to put it into a general EarlyVisionModel class.

Some of the other examples you give make me think we are lacking one more concept - some sort of standardized definition of 'inputs' and 'outputs' for each level to make mixing-and-matching components easier.

[philippjfr]

I don't quite see how a 'inputs' and 'outputs' based system would address the issues with the current class inheritance based system, since replacing lower-level components will always be difficult when using class inheritance. So I do agree we should eventually come up with a completely modular system.

I guess rather than working through inheritance we'd have different modular classes, which can be combined in a Model object. Each level would be its own instantiated object, which would compute it's parameters in the constructor and would have to specify a matchconditions method and the required number of connect methods using the same decorator as currently. These modular components are then combined in a distinct model class, which handles setting up all the connections between (and probably even within) the modular components. So the Model class would accept a sensory component and any number of processing modules, which it then attempts to resolve all the matchconditions on to connect them up as desired. Not sure what this would mean for setting parameters via the commandline since they would no longer be global but rather be split across the different modules.

So basically you'd have to different base classes:

class ModelComponent(param.Parameterized): 
    """ 
    Model components provide the specification for instantiating a
    set of similar sheets and basically replace the earlier level
    concept. They implement the creation of a number of SheetSpec
    objects either during instantiation or when being called by the
    model class. They also have to implement a matchconditions method,
    which the Model class can use to determine the correct
    connectivity. Finally they can implement any number of connect
    methods (labelled using the Model.connect decorator), which
    compute the parameters for the projection, which is also called by
    the model class once it has resolved all the connections using the
    matchconditions.
    """

    level = param.String()

    sheet_type = param.ClassSelector() ....

    def __init__(self, **params): 
        """
        Implements creation of the sheet spec.
        """

   def setup_params(self, properties):
       """
       Computes the parameters for the sheets of this model component.
       """

   def matchconditions(self, properties): 
       """ 
       Implements match conditions for projections within and outside
       of component.
       """

    @Model.connect('Afferent', projection.SharedWeightCFProjection)
    def _specify_lgn_afferent_projection(self, proj):
        """
        Computes the parameters used to instantiate all Afferent
        connections.
        """

class Model(param.Parameterized):
    """
    The model class can contain any number of ModelComponent objects,
    which it will resolve and instantiate the connections between. It
    could either treat all components the same, i.e. have a single
    component list, or be more explicit requiring at least on
    input/sensory component and any number of subcortical and cortical
    components. It also takes care of coordinating parameters across
    the different components and set up global defaults (e.g. for
    analysis).
    """

    input_component = param.ObjectSelector()

    processing_components = param.List()

    OR

    subcortical_component = param.ObjectSelector()

    AND

    cortical_components = param.List()

    def __init__(self, **params):
        """
        Ensures that all required components have been specified.
        """

    def setup_attributes(self):
        """
        Initializes attributes on used components correctly whether
        that be response, learning or output_fns or a global way to
        coordinate the individual model components themselves.
        """

    def setup_model(self):
        """
        Goes through all components resolving their internal and
        external match conditions to initialize the appropriate
        projections.
        """

    def setup_analysis(self):
        """
        Sets up the appropriate analysis defaults for the full model
        """

So basically the whole system stays largely the same, but instead of using decorators to specify different levels within one class you simply have different component objects, which are appropriately connected by the model class.

I'm sure I'm glossing over lots of other issues as I haven't been thinking about this for as long as you guys but something like that would make sense to me and would allow for full modularity as Jim has been suggesting. Please tell me if this is completely nuts.

[jlstevens]

I think Philipp's suggestion is interesting and I would be happy to refactor the code like this - but only if everyone supports this idea!

Some additional thoughts/ ideas about how this could work better:

• Right now the levels are just strings declared across the class in various places. Although chosen by the modeller, these strings are crucial (for connecting everything together properly).
• I feel a more well defined interface for defining these strings is needed, especially if you wish to factor things out into components as Philipp suggests. The essential problem is that the level strings must be kept consistent across components to make connections between components.

• One solution to this is to declare a list of level names that each component supplies and another list of the levels the components requires. An example of these two lists could be:

  supplied_levels = ['V1']
  required_levels=['LGN']

• This would help declare how components can compose together. You may have already spotted that you may actually want something like:

  supplied_levels = ['V1']
  required_levels=['LGN' | 'MGN']

  This would let one cortical component be used for either visual or auditory cortex. Note that only the required_levels attribute might need such disjunctions - I can't imagine a component supplying multiple sets of supplied_levels.

• In the above example, I declared the supplied and required levels as a list of literals. The supplied levels could be generated automatically from the level decorator (which knows all the level names that were used in the class). This would mean the decorators would have to be declared per class (as right now these decorators are effectively global). The required_levels attribute would always need to be stated explicitly.
• Obviously the idea of 'supplied' and 'required' would need to be enforced. When composing components together, each component checks that the previous component supplies the level it actually needs.

• Naturally, I would be tempted to add some nice syntactic sugar to replace Philipp's suggested Model class:

  gcal = EarlyVisionModel() + LGN(gain_control=True) + V1(homeostasis=True)
  gcal()

  The __add__ method would be implicitly creating a Model class. Of course, such a feature would only be implemented at the very end!

Anyway, these are just suggestions - I don't wish to make Tobias panic and assume he has to implement all this stuff immediately. :-)

I will now implement the update to the level decorator as suggested above (declaring the type of the sheet in the decorator).

[jlstevens]

I've just realized that a big problem with my suggestions above is if your input component only refers to 'MGN' and your cortical component refers to 'LGN', you would need to alias those two strings to each other somehow.

There is an even bigger problem with this component based approach: where would you declare afferent projections and their parameters? Neither the LGN or V1 components seem appropriate which suggests you need a "connector" component to join the ModelComponent objects together.

This relates to what I meant by 'inputs' and 'outputs' above - the 'connectors' mediate between the components, standardizing the interface between them. This way, the cortical model can be connected to either an MGN or LGN model without needing to know any details about either.

Anyway, this is all too complicated for now. It seems that unless we get some brilliant idea to keep everything simple, we will be sticking with the current system for now.

EDIT: I realized that we have chosen where to define afferents by convention (as described in the post below).

[jlstevens]

Ok. I've discussed this with Philipp and I think we can keep the system simple while avoiding the problems mentioned above:

• Afferent connections are declared in the component that requires them. This is already the case! This convention makes sense in the current system but it is still just a convention (and one that isn't strictly enforced anywhere).
• Suppose you have some crazy model where the cortical level is connected both to the LGN (or MGN) and directly to the retina. You would then declare the required_levels as follows:

required_levels=['Retina', ('LGN', 'MGN')]

As before this is a list of strictly required components. Any tuples in the list are levels that can be aliased to each other.

• I think all that is now required is to modify the machinery in the Model base class to replace aliased keys with a common alias string. For instance, if the key 'LGN' in encountered, this is replaced with the key 'LGN | MGN' internally. Likewise if the key 'MGN' is encountered.
• This substitution is all hidden from the user/modeler but allows an auditory thalamic model to work with the name 'MGN', a visual thalamic model to work with the name 'LGN' while allowing either to be connected to a cortical model which specifies required_levels=[ ('LGN', 'MGN')].

I hope that all makes sense! I think this could work with the current system without too many changes...

[jbednar]

Some notes:

• Tobias should mostly ignore this discussion -- his current code is sufficient for his project, and he needs to focus on getting color and motion working, and on getting models working together. He's gotten the modelling framework 90% of the way there, satisfying 100% of his own goals, so he needs to start exploiting that to get results for his thesis. Instead, it's now up to Jean-Luc and Philipp -- obviously this new system is going to be a major, major improvement in how we handle the various models that you are working with, and so it makes sense to polish it up now so that it works well for what you need to do. Let's just get the important things that are clearly required, and then gradually tweak it as needed to polish it.
• Yes, GC is an LGN-level concept, and should be implemented solely at that level. I'd favor GC being on by default; we'll mostly want to use it, right?
• Philipp and Jean-Luc, your suggestions for how to make things more modular generally make sense. But I don't think that it's appropriate to declare something like LGN | MGN -- can't you simply say that this GCAL cortical model can accept any input? It's ok to say that it accepts LGN by default, if necessary, but I would think that one could specify at instantiation time that it should be connected to any arbitrary level, which should just work, I'd hope. Certainly it doesn't need to know anything specific about the LGN or MGN, and I don't know of anything about it that would need to change if e.g. it were used for a new V2 with connections from V1.
• The + operator seems reasonable, and covers most cases. For GCALModel it would presumably take anything as input, not checking that it's on some predefined list of ok inputs. Not sure what a good syntax would be for a more complicated tree-like connection structure.

Anyway, if you want to try to implement this more modular approach, great; we can also postpone it for a while until we get more experience with it (though I think the more modular approach will be useful even for the family of models that we are currently using, let alone future ones).

[jlstevens]

I've implemented the changes we agreed to earlier (declaring the sheet type in the level decorator).

In addition, I've added one new class to Lancet (Identity) which has allowed me to greatly simplify setup_sheets. In fact, here is the most complicated example of setup_sheets now:

 def setup_sheets(self):
        return {'Retina': self.args['eyes'] * self.args['cones'],
                'LGN':    (self.args['polarities'] * self.args['eyes']
                           * (self.args['SFs'] + self.args['opponents']))}

You can either return the dictionary of specifications format or Lancet Args objects as I have done above. It made sense to support Lancet Args as Tobias was already using them to define the various cross products for the sheet properties.

On a side note, wouldn't it make sense to add a properties attribute/parameter to actual Sheet objects? Seems like it would be more consistent and maybe even useful!

[jlstevens]

I'm wondering if this Issue should be split into two... one for small improvements to the current system that can be implemented in the short term and a second issue where we can discuss plans for further modularization.

If you agree, lets keep this issue for discussing smaller improvements to the current system and please do open a new Issue!

Lastly, GitHub gives us lots of power to edit existing posts so we can easily move what we have already written to a new Issue fairly easily... The problem is that I don't want to be the owner of the posts copied across!

[jbednar]

The changes using Lancet sound good. Adding properties to sheet makes sense to me; Tobias and I had long-ago discussed whether to add anything to topo.sim's actual objects, and he didn't want to do anything there for now, but eventually I think we should consider it.

[jlstevens]

As for making things modular, we should at least have a plan now before we go down some path and regret not doing things properly to start with.

I think thing are now a little clearer for me and I understand the problem better - in the end, declaring levels that are required is not necessary (but it declaring the levels supplied would be nice to do anyway). Each component creates whatever levels it needs and everything should be fine as long as there are no name clashes.

When it comes to creating connections between components, it all comes down to the matchconditions. For instance, here is what we currently have now in the gcal.py file:

@Model.matchconditions('V1')
        return {'AfferentV1On': {'level': 'LGN', 'polarity': 'On'},
                    'AfferentV1Off':{'level': 'LGN', 'polarity': 'Off'},
                    'LateralV1Excitatory': {'level': 'V1'},
                    'LateralV1Inhibitory': {'level': 'V1'}}

The use of 'level' as part of the match condition is what is tying the GCAL V1 model to an LGN (a visual model) as opposed to some other thalamic pathway. But there is no strict need to include 'level' in the matchcondition property!

For instance, I know next to nothing about the subcortical auditory pathway, but say there are ON and OFF sheets (certainly plausible for at least some non-visual modalities). Then the ''AfferentV1On' matchcondition could be simplified to:

'AfferentV1On': {'polarity': 'On'}

The trick then is to decide what properties are needed to allow 'higher' components to connect to the correct sheets in a general way. I'm not sure what conventions we could pick would be general and make sense so that models can be mixed-and-matched nicely...

On a related note (something Philipp pointed out), shouldn't the projection names be derived from the properties on the input sheet? Here 'AfferentV1On' is declared explicitly but the 'On' suffix is simply the 'polarity' of the input sheet which is just another property like any other. Why the special treatment? Shouldn't projection names also be auto-generated like sheet names are?

[jbednar]

I agree, projection names should be auto-generated just like sheet names. Won't this nicely remove the need to specify anything about the incoming level? I.e., right now it specifies both level LGN and polarity On and the name AfferentV1On; instead it seems like we should just tell GCAL V1 to get connection Afferent from the LGN on instantiation (via the + operator if appropriate), at which point it sees that level LGN has On and Off sheets, and thus creates two projections, AfferentV1On and AfferentV1Off. Seems like as long as a level can define Afferent projection criteria very generally, it will be able to be mixed and matched easily, without any special machinery needed...

[jbednar]

I.e., can't the afferent match condition for all sheets by default, and for V1 sheets in particular, just be 'Afferent': {}? So when connected from any incoming level, by default, just add a connection from every sheet at that level, making connections like LGNOnAfferent, LeftLGNOffAfferent, etc.? Even the LGN's own afferent connections don't need to care about the input level; there needs to be a separate projection defined for On and Off, but after that it would just get duplicated for every sheet in the input level. There's a slight complication for color inputs because of ChannelsGeneratorSheet providing multiple input targets from the same sheet, but that can presumably be dealt with easily by treating a ChannelsGeneratorSheet just as if it is a set of sheets (one per port provided), at least for the purposes of connecting to the next level. I think this all will work out nicely...

[Tobias-Fischer]

We are talking about two different names here. AfferentV1On is a matchname, and the corresponding projection name is returned in the method decorated with @Model.connection('AfferentV1On', projection.CFProjection), i.e. def V1_afferent_projections(self, proj)! Furthermore, I don't see how parameters for these projections would be set if there was no special machinery. Think about motion - how would you do something like 'delay':LGN_V1_delay+lag,?

[philippjfr]

Yes that all sounds like it will work. It should be possible to tie together the LGN component and the V1 component via the + operator and an empty match condition but I'm not entirely sure that's sufficient. I guess it would still work for feedback connections since they are usually more specific and aren't all-to-all like the Afferent projections, which means for a LGN -> V1 -> Layer6 -> LGN feedback loop you'd have to supply something like a 'CorticothalamicFB': {'level': 'Layer6'} matchcondition anyway. Could someone with a better overview comment on how FB connections and circular graphs, such as in a LGN -> V1 -> V1Layer6 -> RTN -> LGN, would work in such a setup? Would we need something like a mapping of expected feedforward and feedback connections?

Re: Tobias, I don't think anyone is suggesting we abandon the parameter setting machinery, all the decorated connect(ion?) methods would still exist but you're clearly correct about the projection naming.

[jlstevens]

This discussion has exploded and my call to create a new Issue (specifically about modularity) seems to have been ignored...

In the distant past, this issue started with me lamenting the lack of tab completion. This and many other details and smaller TODO items have been mentioned without any follow up. For this reason, I've created a wiki page to keep track of these suggestions here. I believe Tobias has write access and he should complain if he doesn't!

This particular issue has been hijacked at this point so I've updated the title and made a new issue specifically about tab completion #574. We may have to create additional issues as new ideas crop up.

[philippjfr]

Been thinking a bit more about the plus operator syntax and it really ends up either being too restrictive or potentially confusing. Addition really implies a linear chain of modules being added together, which means that branching or looping of modules would confuse the semantics and enforcing linear chains would be too restrictive. I think we're fairly close to coming up with an alternative, which may also allow us to simplify match conditions a little bit.

[jbednar]

Right; even if addition was really useful as a syntax, there are so many cases when it doesn't work that there would still need to be another also-not-that-bad way to specify how to connect the levels...

[jlstevens]

Firstly, I think any component will probably need to some something about what it is connected to. For instance, there may be multiple, distinct types of afferent input that might be appropriate for different projections connecting to different sheets. The essential problem is that a component shouldn't have to define matchconditions using the properties internal to some other component.

For this reason, I think it might be possible to keep it simple - each component should declare some named matchconditions for other components to make use of, even if that component doesn't use those matchconditions itself.

For instance, both an LGN and MGN component could declare a 'thalamocortical' matchcondition for a cortical component to use. Then it would be up to the LGN and MGN to choose the properties that would make sense for an outside component to connect to. Of course a component could offer multiple named matchconditions for other components to use as necessary.

In short, I think if components declare 'public', named matchconditions for other components to use, we could go some way to solving this issue. Hope this makes some sense! What to you think?

[jbednar]

I agree that the solution is for levels to declare something about that they provide and what they accept, but I would think any name like "thalamocortical" is specifying too much and is too limiting. E.g. if I want to add an LGN made of simple relay cells to GCAL, I should be able to connect it to the already available On or Off model, and to connect the already available V1 model to the relay cells, in turn, without changing either the On or Off level or the V1 level -- there should just be some generic "driving", "feedforward", or "afferent" output from most ordinary levels, and some generic input with the same name in most ordinary levels, that can each be interconnected. Other types of input and output can then be defined, such as "feedback" or maybe "divisive_feedback", and then the feedforward/afferent input would ignore those, only connecting to those labeled as being suitable.

Philipp would have a better idea of which other types of interconnections we need to handle, but it seems to me that this would take care of our needs and be extensible in the long run -- handle the common feedforward case in a standardized way, handle lateral connnections within levels without worrying about standardization since that's within a level, and then allow anything else to happen using special conditions that are more specific. As we work out good models for various layers, etc., we can develop standardized nomenclature for them as well, but for now surely we should have a standard way to handle the feedforward pathway!

[philippjfr]

I anticipated you saying that when I initially suggested thalamocortical but there's reason why getting as generic as "feedforward" or "afferent" is at the very least insufficient. Let's say we have a Retina -> LGN -> V1 -> V2 chain where V2 is basically identical to V1. When we were still considering the plus operator this would have been fine since each component would know what the previous one is in this chain, so V1 can simply get all "afferent" projections from LGN and V2 all afferents from V1. However, now that we will probably abandon the plus operator there is nothing to indicate whether V2 should get its afferents from LGN or V1.

I also don't quite see how we can make things quite as generic as you'd like for other reasons. Does it really make sense to reuse a V1 component as V2? To get a V2 that makes sense you'd at the very least have to tune the CF bounds. We could introduce something like a cortical level, which could then be used to a) scale the CFs and b) indicate the hierarchy of connections, however really you'd want to perform independent spatial tuning of the V1 and V2 model component. Basically I don't see how we can use names as generic as "Feedforward" without some other way of indicating the graph structure.

Now that I think about it, an overall level number might actually work. I'll try to through a complicated model to show how, Retina (Lvl 0), LGN (Lvl 1), Superior Colliculus (Lvl 2), V1 (Lvl 2), V2 (Lvl 3). Now each level would know to receive feedforward input from the lower level and if desired also from the same level. Similarly feedback connections can be received from the same level or higher level. This way any graph structure can be specified, including branching an looping.

This is what the model structure here could look like, if LGN, V1 and V2 specify lower levels only as their input. SC on the other hand would specify that it receives FF input from it's own level or lower and feedback from any level higher than itself.

Retina ----> LGN -----> V1 -----> V2 ......................|...............|..............| ......................---------> SC <--------

Does that make sense?

[jlstevens]

I think something like a 'thalamocortical' matchcondition might be too general in some circumstances.

What about species where the konio afferent pathway projects to a distinct layer of V1, for instance? Either you need a 'thalamocorticoK' matchcondition declared in the LGN component, or maybe you could add a 'pathway':'konio' to a more general matchondition in the V1 component. The problem with the latter approach is that it involves knowing the properties of a previous component (exactly what we are trying to avoid!).

I agree with Philipp that a lot more generalization would be needed to allow one cortical model to act as V1 and V2 for example. As for the + operator, I also agree that this cannot imply connection ordering because it wouldn't allow the graph structure to be specified (it does not explicitly suggest any directionality)

My guess is that you can specify any graph structure we want with only one 'connect' operator (here I chose >>) and one 'union' operator (here I chose |) as follows:

retina, lgn, v1 = Retina(), LGN(), V1()
retina >> lgn >> v1 | (v1 >> lgn)

The >> operator implicitly suggests the connection direction and the | operator allows you to join connection declarations together to create a single model object. Although this syntax could go some of the way towards making the names more general, you might still need specific matchconditions (pathways, receptor types etc).

[jbednar]

Sure, even if we abandon the plus operator, we need to have something to indicate that a given level should connect to another level. I strongly feel like that something should be in the .ty file, at instantiation time, even if there is some default somewhere in topo.submodels. That's the only way we will be able to mix and match and set up things easily. Sure, we'll have complicated cases, and we'll eventually have a lot of differences between models of different regions. But the proccess of setting it up should be simple to start with, only requiring new code for new functionality, and ideally we should be able to handle a very wide rang of such differences at instantiation time, not at Model class definition time. Or at least we should have the option to seamlessly move between the two, initially choosing things at instantiation time and then gradually codifying them as they get more agreed upon and/or more complex.

The University of Edinburgh is a charitable body, registered in Scotland, with registration number SC005336.

[jlstevens]

I strongly feel like that something should be in the .ty file, at instantiation time, even if there is some default somewhere in topo.submodels. That's the only way we will be able to mix and match and set up things easily.

Agreed! The plus operator isn't really being abandoned though as it was never actually implemented. :-)

I would like to find out if we are in agreement about these points:

• Each component should make matchconditions available to outside components via easy-to-use labels.
• The + operator is insufficient but a combination of ">>" (maybe ">" instead) and "|" might work to specify the network structure.
• The simple case should be simple but in more complex situations, knowing some of the details of the input components will be unavoidable.
• It looks like we will be able to achieve what we want based on the existing system without a major rethink.

I'll be working on my proposed improvements to the decorators (and tab-completion) today but we need a plan for what is worth doing in the short-term and what isn't.

[jbednar]

Agree with all that except that I have no opinion on >> and |; seems like anything in a single expression will fail to express recursion.

The University of Edinburgh is a charitable body, registered in Scotland, with registration number SC005336.

[jbednar]

| retina, lgn, V1 = Retina(), LGN(), V1() | Retina() >> (lgn >> v1) | (v1 >> lgn)

Assuming you mean "retina >> (lgn >> v1) | (v1 >> lgn)", I'm not sure how to interpret the value of these subexpressions. But if it's at all difficult, we don't need to support defining the graph scructure in a single expression, do we? E.g. something expressed fully like:

photoreceptors.efferent >> rgc.afferent
rgc.efferent >> lgn.afferent
lgn.efferent >> v1.afferent
v1.efferent >> lgn.modulate

could omit the default .efferent (for lhs of >>) and .afferent (for rhs of >>) for readability:

photoreceptors >> rgc
rgc >> lgn
lgn >> v1
v1 >> lgn.modulate

and could presumably be chained for convenience:

photoreceptors >> rgc >> lgn >> v1
v1 >> lgn.modulate

I.e., use >> to allow any connection ever, and use chaining to support a simple feedforward pathway (with local recursion defined by individual levels as usual), without restricting anthing to be feedforward only.

Each level would then define detailed match conditions based on properties, without reference to the level at the other end by its actual name, just generically as "whatever is connected to my efferent (or afferent) port". Things would only get connected if they matched the properties appropriately, to any level of precision that we wish, but they don't need to do so by naming the level at the other end, but by talking about properties of whatever it is that's connected.

I'm sure there are things I'm not thinking clearly about, but I do think that we should be able to get it to work without having most levels know anything about the identity of what they will connect to, even if they do need to know about its properties.

The University of Edinburgh is a charitable body, registered in Scotland, with registration number SC005336.

[jlstevens]

I've made some rather long commits to implement tab-completion as described in #574 that only implement some nice syntactic sugar while leaving the core system the same.

| retina, lgn, V1 = Retina(), LGN(), V1() | Retina() >> (lgn >> v1) | (v1 >> lgn)

Don't you mean?

| retina, lgn, V1 = Retina(), LGN(), V1() | retina >> (lgn >> v1) | (v1 >> lgn)

If the original version looks ambiguous to you, feel free to add brackets wherever you like!

Personally, I think it is clear enough, the right side of >> only connects directly to the left side unless otherwise specified. For instance, I think it is relatively clear what this expression would do:

retina >> lgn >> v1 | v1 >> lgn

Whereas this expression would allow V1 to connect directly to the retina:

(retina >> lgn) >> v1 | v1 >> lgn

I am very much against defining the model across multiple expressions via side-effects like your example - I want one clear declarative expression for what the model is. In practice, the only difference is adding the | operator between all your statements!

Otherwise, I mostly agree with your suggestion but I do think there is one key difference - I consider the point of the named match conditions is to avoid reference to any properties in an external component, not just the name of the 'level' property.

Your suggestion would work, but only if there is a public interface declaring which properties are defined for use by other components (and what values they can take).

Hope that makes sense!

[jlstevens]

I've just created a new pull request using the simplified decorators I suggested this morning: #575

[jlstevens]

It has become clear from our discussion that increasing modularity with a component based approach is really quite tricky with lots of subtle issues to worry about.

For this reason, I have summarized our discussion on the wiki and feel we can now close this issue.

If there is anything specific that can be addressed in the short-term we can open new issues as necessary.