Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor files, emulator, api #159

Merged
merged 4 commits into from Jan 31, 2019
Merged

Refactor files, emulator, api #159

merged 4 commits into from Jan 31, 2019

Conversation

hunse
Copy link
Collaborator

@hunse hunse commented Dec 21, 2018

Replacement for #30. Currently based on #139, #124, #132.

The main file moving around has been done in the first commit. I tried to change as little as possible with the code (other than fixing imports), so if we do have to rebase this, it should be a simple matter of copying the changes to the right place.

TODO:

  • Refactor emulator
  • Integrate hardware api.py/allocator.py/builder.py. I'd be nice to perhaps have some logical distinctions here, but I'm not sure if the current divisions make sense.
  • Renaming things (e.g. all the Cx classes lose the Cx)
  • Add *Group classes (e.g. AxonGroup, SynapseGroup), and split up the discretize function
  • Some minor changes to simulator.py (e.g. using iteritems). Do we want to worry about iteritems? I feel like the performance benefit is very very minimal unless we've got huge dictionaries, and of course the benefit would only be for Python 2 (Python 3 is already fast).
  • Find a good place for test_model.py. It currently only has one test (the noise test) in it. This test also needs to be fixed so that it asserts something (currently only a visual test).
  • Test everything off-chip and on-chip

Discussion:

  • When do we want to do validation for chip constraints? Previously, we had done this all together when the Model gets passed to the emulator or hardware builder. Rearrange main classes, refactor emulator #30 started to do some of this when we add objects (e.g. Synapses) to their respective containers (e.g. SynapseGroup).
  • Rearrange main classes, refactor emulator #30 introduced a CoreGroup, which would house a CompartmentGroup along with associated AxonGroup, SynapseGroup, and ProbeGroup. I haven't done that yet for this PR, instead opting to keep AxonGroup, SynapseGroup, and ProbeGroup as attributes of CompartmentGroup. This fits better with my mental model, since a CompartmentGroup can potentially span multiple cores (though we don't support this yet), and the Synapses, Axons, and Probes all exist in relation to particular compartments.

@hunse
Copy link
Collaborator Author

hunse commented Jan 3, 2019

I was just thinking about how this structure might change if/when we want to have multi-compartment neurons. In this case, maybe we want a NeuronGroup class where you can specify a certain number of neurons and a neuron model. The neuron model would define how the neuron is created using one or more compartments. The NeuronGroup could then be like CoreGroup in #30, and own a CompartmentGroup, AxonGroup, SynapseGroup and ProbeGroup. Does this structure make sense? (Basically, it's the same as #30, except that CoreGroup is called NeuronGroup instead, and we have the explicit plan of expanding it for multi-compartment neurons.)

@tbekolay
Copy link
Member

tbekolay commented Jan 7, 2019

When do we want to do validation for chip constraints?

I'm cool with deferring it until model build to reduce the number of changes here.

CoreGroup vs NeuronGroup

NeuronGroup is better, CoreGroup was a name that I was like "hopefully I'll come up with something better later" because it's the wrong abstraction level anyway (those objects aren't yet tied to a chip location, really).

@hunse
Copy link
Collaborator Author

hunse commented Jan 7, 2019

Based on our naming for compartments, probes, etc., NeuronGroup would fit best in a file called neurons.py. However, we already have one for neuron types. Where should NeuronGroup go? We could rename the current neurons.py to neurontypes.py, but this doesn't fit with Nengo (maybe that's not the end of the world, though).

@tbekolay
Copy link
Member

tbekolay commented Jan 7, 2019

We could name it EnsembleGroup, put it in ensemble.py, that's maybe more consistent for a few reasons.

@hunse hunse force-pushed the refactor-files branch 3 times, most recently from b4b3c8a to 9f027a8 Compare January 11, 2019 16:12
Copy link
Member

@drasmuss drasmuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor comments, definitely like these changes a really nice improvement to the organization 👌

nengo_loihi/builder/builder.py Outdated Show resolved Hide resolved
nengo_loihi/emulator/emulator.py Outdated Show resolved Hide resolved
nengo_loihi/hardware/simulator.py Outdated Show resolved Hide resolved
nengo_loihi/inputs.py Outdated Show resolved Hide resolved
nengo_loihi/emulator/emulator.py Outdated Show resolved Hide resolved
nengo_loihi/emulator/emulator.py Outdated Show resolved Hide resolved
nengo_loihi/compartments.py Outdated Show resolved Hide resolved
nengo_loihi/probes.py Outdated Show resolved Hide resolved
nengo_loihi/synapses.py Outdated Show resolved Hide resolved
nengo_loihi/axons.py Outdated Show resolved Hide resolved
@drasmuss
Copy link
Member

Rebased this to master (saved the previous history in refactor-files-saved, in case I messed something up).

@drasmuss drasmuss assigned drasmuss and unassigned drasmuss Jan 16, 2019
@drasmuss drasmuss removed their assignment Jan 16, 2019
@drasmuss
Copy link
Member

Note: all emulator tests passing, but there is still one test failure on hardware (test_emulator.py:test_uv_overflow[1000]) that I'm trying to figure out.

@drasmuss
Copy link
Member

All tests passing now on hardware and emulator.

@hunse hunse force-pushed the refactor-files branch 4 times, most recently from cc1ca2b to 3e212c7 Compare January 21, 2019 05:36
@hunse
Copy link
Collaborator Author

hunse commented Jan 21, 2019

This is all rebased to master, and I think all comments are addressed. Hopefully all the tests pass.

Copy link
Member

@tbekolay tbekolay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a bunch of inline comments, some of which might need some discussion, so possibly we should make a call for that?

Another thing that comes to mind in general is that it's not clear to me the distinction and hierarchy of CompartmentGroup and NeuronGroup. Which contains the other? I know we already discussed the name NeuronGroup, but maybe we should revisit that in the meeting as well.

nengo_loihi/hardware/simulator.py Outdated Show resolved Hide resolved
nengo_loihi/builder/builder.py Show resolved Hide resolved
nengo_loihi/builder/connection.py Show resolved Hide resolved
nengo_loihi/builder/decode_neurons.py Outdated Show resolved Hide resolved
nengo_loihi/builder/ensemble.py Show resolved Hide resolved
nengo_loihi/hardware/api.py Outdated Show resolved Hide resolved
nengo_loihi/hardware/builder.py Show resolved Hide resolved
nengo_loihi/inputs.py Outdated Show resolved Hide resolved
nengo_loihi/neurongroup.py Outdated Show resolved Hide resolved
nengo_loihi/utils.py Outdated Show resolved Hide resolved
@hunse
Copy link
Collaborator Author

hunse commented Jan 21, 2019

For CompartmentGroup vs NeuronGroup: a CompartmentGroup is a member of a NeuronGroup. Right now, the number of neurons is the same as the number of compartments, but the idea is that when we have multi-compartment neurons, you'd create a neuron group and say e.g. I want 100 neurons of type AlphaNeuron. If AlphaNeuron has three compartments, then the CompartmentGroup will have 300 compartments that implement the 100 neurons.

For some reason, I can't "reply" to some of the inline comments, so I will here. Re: renaming LoihiSimulator: The nice thing about Emulator is that it makes it clear both where the thing is (not on hardware) and what it does (emulates the hardware). My problem with just Hardware or Loihi is both of those say where it is, but they don't say what it does (simulates things on hardware/Loihi). I don't think it's the end of the world, so if we're going with one of the two, I guess Hardware? (I like the name hardware for the folder, and I agree having them be the same is nice. Having a loihi folder seems a bit confusing, because everything is Loihi-related in this project, but again, I'm not too opinionated on that.)

@hunse hunse force-pushed the refactor-files branch 2 times, most recently from 20ba2f6 to 07a7a36 Compare January 24, 2019 15:25
@tbekolay
Copy link
Member

We discussed a few ways to organize files yesterday, all with some pros/cons. It's easier to compare the options if we can see the final source tree, so here are those options.

Option 1: Loihi-oriented 😄

Couldn't think of a good name, but this is the current state of this PR. File names are roughly based on class names, which are tied to how the chip works.

In this and subsequent file listings, I've omitted test files and __init__.py files.

Here I've annotated with the # of lines of code, as we know this from the current branch. In subsequent options, I'll estimate the number of lines of code given what I assume will go in each file.

nengo_loihi
├── builder
│   ├── builder.py (190)
│   ├── connection.py (331)
│   ├── decode_neurons.py (311)
│   ├── ensemble.py (161)
│   ├── node.py (19)
│   └── probe.py (148)
├── emulator
│   └── emulator.py (667)
├── hardware
│   ├── allocators.py (128)
│   ├── builder.py (497)
│   ├── nxsdk_objects.py (371)
│   ├── nxsdk_shim.py (33)
│   └── simulator.py (431)
├── axons.py (139)
├── compartments.py (271)
├── config.py (46)
├── conv.py (715)
├── discretize.py (186)
├── io_objects.py (131)
├── neurongroup.py (65)
├── neurons.py (199)
├── passthrough.py (274)
├── probes.py (47)
├── simulator.py (753)
├── splitter.py (470)
├── synapses.py (500)
└── version.py (15)

Option 2: Nengo-oriented 🎉

This is roughly what I was thinking about as a possible Nengo-oriented file structure when I was reviewing the PR last week.

nengo_loihi
├── builder
│   ├── builder.py (190)
│   ├── connection.py (331)
│   ├── decode_neurons.py (311)
│   ├── ensemble.py (161)
│   ├── node.py (19)
│   └── probe.py (148)
├── emulator
│   └── emulator.py (667)
├── hardware
│   ├── allocators.py (128)
│   ├── builder.py (497)
│   ├── nxsdk_objects.py (371)
│   ├── nxsdk_shim.py (33)
│   └── simulator.py (431)
├── connection.py (639 = 139[axons.py]+500[synapses.py])
├── config.py (46)
├── conv.py (715)
├── discretize.py (186)
├── esnemble.py (336 = 271[compartments.py]+65[neurongroup.py])
├── io_objects.py (131)
├── neurons.py (199)
├── passthrough.py (274)
├── probe.py (47)
├── simulator.py (753)
├── splitter.py (470)
└── version.py (15)

Option 3: File suffixes, Nengo-oriented ❤️

This could also be prefixes, but I think suffixes are preferred. This is similar to how NengoDL is organized. Here, I combine some of the top-level files into the _builders.py files. This makes some of them quite long (notably connection_builders.py) but I think that once we put them together, we might be able to see some opportunities to refactor further, e.g. to put learning rule related things from connection_builders.py into learning_rule_builders.py.

nengo_loihi
├── emulator
│   └── emulator.py (667)
├── hardware
│   ├── allocators.py (128)
│   ├── builder.py (497)
│   ├── nxsdk_objects.py (371)
│   ├── nxsdk_shim.py (33)
│   └── simulator.py (431)
├── builder.py (190)
├── connection_builders.py (970 = 331+500[synapses.py]+139[axons.py])
├── config.py (46)
├── conv.py (715)
├── decode_neurons.py (311)
├── discretize.py (186)
├── ensemble_builders.py (497 = 161+271[compartments.py]+65[neurongroup.py])
├── io_objects.py (131)
├── neurons.py (199)
├── node_builders.py (19)
├── passthrough.py (274)
├── probe_builders.py (195 = 148+47[probes.py])
├── simulator.py (753)
├── splitter.py (470)
└── version.py (15)

There might also be an option 3b or 4 that is a variant of this option that keeps axons.py, but synapses.py is a bit tricky because in this case we would expect synapses.py to contain Synapse subclasses.


Vote with emoji reactions if this all makes sense, otherwise comment away!

@hunse
Copy link
Collaborator Author

hunse commented Jan 24, 2019

Option 5: Loihi-oriented with folder 🚀

Like option 1, but moves some files into an api folder (the files with classes that describe the mid-level Loihi model). I could also see this folder being called loihi or model. This means that none of the files in the root directory share names with nengo files but deal with different objects.

nengo_loihi
├── api
│   ├── axons.py (139)
│   ├── compartments.py (271)
│   ├── decode_neurons.py (311)
│   ├── neurongroup.py (65)
│   ├── probes.py (47)
│   └── synapses.py (500)
├── builder
│   ├── builder.py (190)
│   ├── connection.py (331)
│   ├── decode_neurons.py (311)
│   ├── ensemble.py (161)
│   ├── node.py (19)
│   └── probe.py (148)
├── emulator
│   └── emulator.py (667)
├── hardware
│   ├── allocators.py (128)
│   ├── builder.py (497)
│   ├── nxsdk_objects.py (371)
│   ├── nxsdk_shim.py (33)
│   └── simulator.py (431)
├── config.py (46)
├── conv.py (715)
├── discretize.py (186)
├── io_objects.py (131)
├── neurons.py (199)
├── passthrough.py (274)
├── simulator.py (753)
├── splitter.py (470)
└── version.py (15)

@tbekolay
Copy link
Member

The thing that I find confusing about Option 5 is what differentiates things in the api or model folder from things in the root folder. Why do decode_neurons.py go there, but not io_objects.py or discretize.py ? And if we're calling it the API or the underlying model, who are we expecting to use it? I see the user-facing API as being mostly nengo_loihi.Simulator and the things in config.py and neurons.py.

Since we have two for option 3, are there specific things that you don't like about it that we can change @hunse?

@hunse
Copy link
Collaborator Author

hunse commented Jan 24, 2019

I too was struggling with what exactly goes in the api folder. Since it's called api, I was thinking it's all files that hold objects associated with the mid-level Loihi model. discretize.py has many functions associated with this API, but also has functions used elsewhere, which is why I left it out. io_objects.py are mostly based off Nengo objects and used in splitter.py, so I kept it out.

And if we're calling it the API or the underlying model, who are we expecting to use it?

It's the Loihi API that developers use to write models for the chip. Not all APIs have to be user-facing, in my opinion, but I can understand how just calling it api implies this. I'd be fine with loihi_api, or just loihi. Or some other name.

The biggest thing I don't like about option 3 is it hides this mid-level Loihi API (or whatever you want to call it). To me, this is the core around which nengo_loihi is built, and for anybody wishing to do considerable work on the project, I think it's the first thing that they should understand. To me, this API encapsulates how the chip is organized, without getting into the nitty-gritty. I feel like option 3 hides it and makes it more difficult to understand, which may seem better at first (since it hides complexity) but in the long-term is worse for somebody understanding the project.

I also don't like that option 3 doesn't have a builder folder. This differs from nengo and is unexpected, and also makes for unwieldy filenames IMO.

It's also just different from the way I naturally group things. I tend to group objects at the same level of a hierarchy together, even if they're serving different functions. Option 3 groups things more functionally (e.g. everything related to connections together) grouping across levels in the hierarchy. There's no clear right or wrong, it's just not the way I tend to group things in my mind.

@tbekolay
Copy link
Member

  1. What about LoihiGroup instead of LoihiSegment? Objectively, they're about equally descriptive I think (for somebody new joining the project), and group is shorter and fits with our current nomenclature.

The reason I changed away from Group is because it doesn't really position LoihiGroup separate from Axons, Synapses etc, it feels like it's on the same level, where I want to be explicit that it is a higher level because it contains those things. So the way I did that was to instead position it more on the Board, Core, Chip level where you're thinking about the Loihi system, and the LoihiSegment is a somewhat more abstract (i.e., not actually tied to hardware) version of that, where we're saying "this is some chunk of a board/core/chip" instead of saying "this is a collection of axons, synapses, compartments and probes." It's kind of equivalent for us once we realize the group/segment is a higher level abstraction, but for newcomers it is not easy to realize that fact.

Also, LoihiGroup doesn't necessarily do a good job saying what it is a group of. Are LoihiInputs part of the group? There's no reason why they couldn't be. With LoihiSegment, since we're saying that this is a chunk of the hardware, then an input doesn't necessarily make sense there because an input is how we interface with the board, it isn't part of the board itself.

Not saying that these abstractions are perfect and 100% clear, just explaining my motivations for moving away from the Group name. Would be best to get another opinion on it.

Also, why the name switch back to inputs.py from io_objects.py?

Yeah I realized after I switched it that it used to be inputs. Like I said earlier, the key thing was that a Nengo Loihi model consists only of a set of LoihiSegments and a set of LoihiInputs. We call it inputs in the model, so that should be reflected in the file structure. When I was cool with the switch to io_objects.py that wasn't my mental model.

The output part of it is a bit tricky since we don't really have explicit things to shuttle data off the chip. I think that we should, it would make the backend splitter / passthrough stuff more clear, but since we haven't done it yet putting it inputs.py seems OK because we will want to have that file around permanently, whereas the compromise io_object.py I would envision being split up into inputs.py and outputs.py eventually.

I'd be fine switching everything to singular for the class names.

Me too, seems easier to remember that all are singular vs some singular, some plural.

I'm still not convinced that having connection_builders.py etc. is better than having a builder folder

My concerns were more around the other files, I'm fine going back to a builder folder for the suffixed files 🤷‍♂️ So unless @drasmuss has strong objections let's do that.

@hunse
Copy link
Collaborator Author

hunse commented Jan 28, 2019

Also, LoihiGroup doesn't necessarily do a good job saying what it is a group of. Are LoihiInputs part of the group? There's no reason why they couldn't be. With LoihiSegment, since we're saying that this is a chunk of the hardware, then an input doesn't necessarily make sense there because an input is how we interface with the board, it isn't part of the board itself.

I think you have more associations with the word "segment" than I do.

What about LoihiEnsemble? Right now, Nengo Ensembles map one-to-one with this class. Their other main use is interneurons, which again are like Ensembles. This would highlight that they should have all the things you'd associate with an Ensemble (synapses, axons) but not inputs (which in Nengo associate with Nodes). So this is more of a Nengo-specific connotation, but would be a great connection for anyone familiar with Nengo, I think.

@drasmuss
Copy link
Member

I think moving things back into the builder folder makes sense, since now those files only contain the build_* functions rather than the midlevel objects as well.

No strong opinion on LoihiGroup/Segment, either one feels about equally meaningful to me. The thing I worry about calling it LoihiEnsemble is that I'd then expect there to be a LoihiConnection as well, whereas in the loihi mental model those things are kind of amalgamated.

@tbekolay
Copy link
Member

I think you have more associations with the word "segment" than I do.

I mean, I didn't before I switched things and thought about it a bit, so it's not like a prior association I had. I tried to explain my reasoning so we'd all come to a shared understanding about things. If my thought process doesn't give you similar associations with segment, then we should think of something better, I just have trouble coming up with something else.

What about LoihiEnsemble?

The main problem here, aside from what Dan pointed out, is that it would go in ensemble.py, which we already said that we should avoid.

@tbekolay
Copy link
Member

If my thought process doesn't give you similar associations with segment

Also spending some time using it helps, it's hard to think about abstractly... using group to this point didn't seem to give us all a similar understanding

@hunse
Copy link
Collaborator Author

hunse commented Jan 28, 2019

I mean, I didn't before I switched things and thought about it a bit, so it's not like a prior association I had. I tried to explain my reasoning so we'd all come to a shared understanding about things. If my thought process doesn't give you similar associations with segment, then we should think of something better, I just have trouble coming up with something else.

I think that your reasoning makes sense. All I'm saying is for somebody coming to the project, this isn't going to be immediately obvious. No matter what the name is, we're going to have to explain what it is, what it does, what's in it.

What about LoihiBlock? It basically means the same thing as LoihiSegment, but it's a little more concise, and it has a nice building-block feel to it (since these are essentially the building blocks that we use to make a Loihi model).

@tbekolay
Copy link
Member

I like block, feels more concrete like the board/core/chip, but also shorter 👌

Could you make those changes @hunse?

@hunse
Copy link
Collaborator Author

hunse commented Jan 28, 2019

Ok, I think I've made all the changes.

Copy link
Member

@tbekolay tbekolay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small inline comment, but I'll fix that when I squash/fixup the history.

@drasmuss can you approve if this looks good to you (or if you're cool with whatevs).

nengo_loihi/tests/test_model.py Show resolved Hide resolved
tbekolay pushed a commit that referenced this pull request Jan 30, 2019
tbekolay pushed a commit that referenced this pull request Jan 30, 2019
@tbekolay tbekolay force-pushed the refactor-files branch 6 times, most recently from 385e3f7 to ada3a34 Compare January 30, 2019 18:41
These lines are often long by virtue of long email addresses,
which we do not want to line break because they are automatically
parsed by some tools.
hunse and others added 3 commits January 30, 2019 23:26
Major organizational changes
----------------------------

- Made `builder`, `emulator`, and `hardware` folders to clean
  up the root directory and make clear what is hardware-specific.
  Renamed `loihi_api.py` and `loihi_interface.py` and moved them
  to the hardware folder.
- Moved classes related to Loihi inputs from loihi_cx.py to inputs.py
- Renamed CxSimulator -> EmulatorInterface
- Renamed LoihiSimulator -> HardwareInterface
- Renamed CxAxons -> Axon
- Renamed CxGroup -> Compartment
- Renamed CxProbe -> Probe
- Renamed CxSpikeInput -> SpikeInput
- Renamed CxSynapses -> Synapse
- Renamed hardware.api.SpikeInput -> LoihiSpikeInput
- Made a new LoihiInput base class that SpikeInput inherits from.
  In the future, we plan to investigate other ways of providing
  input to a Loihi model.
- Added LoihiBlock
  - This is a new abstraction above the level of Axon, Compartment,
    Probe, and Synapse that serves as the main container for a "block"
    of the Loihi system (i.e., a chunk, region, slice, or segment).
  - Backend developers should think of the Loihi system (Loihi model)
    as a collection of LoihiBlocks and LoihiInputs.
  - A LoihiBlock can have elements across multiple cores/chips.
  - A LoihiBlock contains one Compartment instance, and any number of
    Axon, Probe and Synapse instances.

Major refactoring
-----------------

- Combined CxModel into Model
- Split the EmulatorInterface class into state components for
  Compartment, Synapses, Axons, Probes.
- Moved discretize and validation methods in Loihi classes to
  `discretize.py` and `validate.py` files. Makes the Loihi
  classes (in `block.py`) easier to read, and reduces the number
  of imports needed when discretizing is done in different places.
- Renamed `Synapses.tracing` to `.learning` for clarity.
- Removed unused group attribute from Axon/Synapse.
- Removed unused location attribute from Compartment.
- Synapse validation now calls SynapseFmt validation.

Other changes
-------------

- Removed "cx" from some, but not all, variable/function names.
- Fixed `test_uv_overflow`.

Co-authored-by: Trevor Bekolay <tbekolay@gmail.com>
Co-authored-by: Daniel Rasmussen <daniel.rasmussen@appliedbrainresearch.com>
Rather than having redundant copies of Nengo code.
@tbekolay tbekolay merged commit 46d7d07 into master Jan 31, 2019
@tbekolay tbekolay deleted the refactor-files branch January 31, 2019 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants