Allow SPA style connection to BG utility input. #194
Conversation
| return as_ast_node(self).translate(vocab, populate, keys, solver) | ||
|
|
||
|
|
||
| class ConnectorRegistry(object): |
There was a problem hiding this comment.
I understand most of this PR, except for this one class which is confusing me. This class associates a connection to a vocabulary? So it's basically a dictionary, but how it adds items is verified such that... This is where I'm lost. What exactly is happening with the types? Would it be helpful to add a comment?
There was a problem hiding this comment.
This code is pretty much copied from the spa.Network.declare_input and spa.Network.declare_output functions and put into a class to have the code only once (instead of duplicated in both methods). It associates a Nengo object with a vocabulary and allows it to be used in the SPA syntax by dynamically changing the class of the Nengo object instance. The new class is also dynamically generated as a class deriving from the original class of the Nengo object and SpaOperatorMixin. Because there is only a limited number of Nengo objects, I don't want to create a new type/class each time and cache them. Furthermore, this ensures that everything with the same type has an identical class.
This is definitely a bit of Python black magic ... so maybe I should add a comment, but not sure yet how to make it concise and helpful.
- Writing this I noticed that the
_type_cacheshould actually be a class variable.
|
Remaining TODOs should have been addressed in the new commits. |
|
One more commit because test coverage declined. |
|
Still good. |
This reduces the amount of duplicated code and provides an interface that is more easily useable outside of a network.
To make connectors independent of the actions module.
Closes #190. This does not allow connections to the basal ganglia input node in this style because this would require to allow SPA style connections to sliced inputs which is currently not implemented.
Motivation and context:
Closes #190. This is similar to allowing scalar inputs to appropriate network inputs. This does not make the second example in the issue
possible. That would require to handle the slicing on SPA inputs which we currently don't do and I'm not convinced that it would be worthwhile to do so.
Interactions with other PRs:
none
How has this been tested?
added a test
How long should this take to review?
Where should a reviewer start?
Follow the commits. This PR moves some things around to prevent cyclic dependencies.
Types of changes:
Checklist: