Splitter refactoring

[arvoelke]

Closes #80. Refactors the splitter logic for adding / removing connections to occur within the builder. This consolidates the connection logic into one place (builder/connection.py) so that backend-relevant logic and objects are manipulated at the same level of abstraction.

Overarching ideas:

• The splitter returns "directives" (i.e., sets of instructions) to guide the builder.
• The builder fills in all three "sub-models" (self, host, and host_pre) in parallel, by delegating the objects to the appropriate builder methods.

Solves the following bugs:

• Splitter fails for sliced connections and probes #205 - Splitter handles sliced probes.
• Passthrough removal fails for sliced probes #206 - Passthrough removal handles sliced probes.
• Simulator has global side-effect of disabling decoder cache #207 - Decoder cache is not globally disabled.
• Improve error message for chip -> chip learning #208 - Improved chip -> chip learning signal message.
• Improve error message for chip -> chip error signal #209 - Improved chip -> chip error signal message.
• Splitter can mutate the original network #211 - Splitter won't mutate original network.

Random improvements:

• split_pre_from_host is no longer O(n^2) (This was a misunderstanding on my part, fueled by the fact the variable named "queue" was actually a stack -- it is now actually a queue)
• __contains__ is no longer O(n)

Bugs that have been identified that are outside the scope of this PR:

• Connections created by splitter and passthrough removal do not respect seeds #210 - Passthrough connections aren’t seeded, and the order is non-deterministic.
• Passthrough removal is too aggressive #212 - Passthrough removal is too aggressive.
• Passthrough removal broken when connecting from passthrough -> host #213 - Passthrough removal is broken.
• precompute=True can find maximal subgraphs and consider disjoint components  #214 - precompute=True could be smarter.

Existing issues that should now be more readily achievable:

• Passing instance config parameters to builder #163 - Passing instance config parameters to builder
• sim.data does not contain chip <-> host connections #154 - sim.data does not contain chip <-> host connections
• Efficient passthrough nodes #145 - Efficient passthrough nodes
• Indicate which parts of a model are running on/off chip #118 - Indicate which parts of a model are running on/off chip
• Expose params and handle splitter-created objects #4 - Expose params and handle splitter-created objects

Other future work:

• Ability to pass in network=None.
• Change assert self.model.dt == dt to an error and then unit test (Simulator dt ignored if model dt specified nengo#1517).

[arvoelke]

I ran @tcstewar's benchmark notebook with:

conditions = [
    'nengo',                                         # the normal nengo backend
    'nengo_loihi(precompute=False, target="loihi")', # using Loihi itself
    'nengo_loihi(precompute=True, target="loihi")',  # using Loihi itself with precompute
]

on both master and this branch.

On this branch, precompute=False and precompute=True gave identical accuracy (but with different speeds) across all benchmarks and so I modified a simple unit test (45eced7) to verify this. The learning benchmarks aren't supposed to work with precompute=True (#214) and so I added a commit (433b489) to check this and improve the error message.

On master, precompute=False and precompute=True gave different results for ['CircularConvolution', 'MatrixMultiply', 'SPASequence'], which I presume has something to do with some interaction between remove_passthroughs=True and precompute with the seeding of the network in the old splitter code (also see issue #210).

[arvoelke]

Pushed two more commits (1711044, 0ca11d8) to move LoihiInput and SpikeInput from builder/inputs.py back into inputs.py and move build_conv2d_connection from conv.py to builder/connections.py. This should now address all your comments @hunse.

[arvoelke]

Added a commit (6c201e0) that increases test coverage since I touched the lines of some un-tested convolution code.

[hunse]

Just a few minor comments. Otherwise everything LGTM. Thanks for doing all those tests against the benchmarks.

[tbekolay]

This is a big improvement, will make future development much easier. I made some changes within the history here, so best to see the commits one-by-one for my suggested changes. There are definitely some discussions points here, so it would be great to get @arvoelke and @hunse's thoughts and a signoff.

1. 5694c27 Is Split a good name? Or should we stick with Splitter, SplitterDirective, or some other option?
2. 15890be Similarly, is PassthroughSplit a good name?
3. ea6d52e Should this be a separate PR or is it OK to include here?
4. 16e322a Is get_seed something we want built into Model? And if so, do we do it now or make a separate issue for it?
5. f64b102 Are BuildErrors sufficient here or do we need to make a new NengoException subclass?

[arvoelke]

I added a couple very minor commits for coverage / sanity. I also rebased onto master again to pick up the testing fixes from the current/rates sync-up.

5694c27 Is Split a good name? Or should we stick with Splitter, SplitterDirective, or some other option?
15890be Similarly, is PassthroughSplit a good name?
ea6d52e Should this be a separate PR or is it OK to include here?

LGTM!

16e322a Is get_seed something we want built into Model? And if so, do we do it now or make a separate issue for it?

I think it's fine as is. A bunch of the weirdness with seeding can be resolved when we eventually drop support for nengo<3. This could end up simplifying a bunch of this code.

f64b102 Are BuildErrors sufficient here or do we need to make a new NengoException subclass?

This LGTM, but we should make a card to go through the rest of the code base? e..g, There are a bunch of ValueError exceptions in hardware/builder.py.

[arvoelke]

I just noticed that the simulator docstring says the default for precompute=True. We should fix that here IMO.

[tbekolay]

We should fix that here IMO.

OK, will do. Running into some issues getting the hardware build to pass so I'll be doing at least one more push anyhow.