Reorganized the AssociativeMemory code. #702
Conversation
xchoo
commented
Apr 17, 2015
xchoo
commented
- Reorganized the AssociativeMemory code. Made the associative memory a nengo.Network rather than just a spa module.
- AssociativeMemory module now takes a vocabulary and list of keys to determine the appropriate vectors to use for the underlying AssociativeMemory network.
|
Made a couple comments on the AssociativeMemory code. I know they weren't introduced in this PR since you just copied them from one file to another, but I figure it's an opportunity to clean things up a little bit. Other than that, looks good to me! (also I assume we would squash all this into one commit) |
|
Can we keep commits that are just moving stuff around separate from ones that actually make changes? It makes it easier to see what's actually changed. |
|
Most of this commit is just moving the AssociativeMemory code into networks. Not much functional changes were introduced in this one. I can make another branch based of this one for the proper changes (Dan's suggestions). |
|
Yeah sorry by "squash all this into one commit" I meant the 3 commits in this PR, not the further changes. I just phrased that very confusingly. |
I didn't mean they have to be part of different branches. I just meant don't squash them in. |
|
So is this waiting on @xchoo to organize his commits? |
|
I don't know how to squash. haha. I was hoping trevor could do that for me. 😸 |
|
Use the Are all three commits on this branched supposed to be squashed into a single one? |
|
Yup! |
jgosmann
force-pushed
the
assoc_mem_reorg
branch
from
51fbf11
to
197b9b9
Compare
|
Squashed. |
| import nengo | ||
| from nengo.dists import Choice, Uniform | ||
| from nengo.networks.ensemblearray import EnsembleArray | ||
| from nengo.networks.assoc_mem import AssociativeMemory as AssocMem |
There was a problem hiding this comment.
Personally, I would just do nengo.networks.AssociativeMemory down below where you actually use this (and make sure the AssociativeMemory network is imported into the nengo.networks namespace). It's just a little clearer to me, because nothing gets renamed, and you're just using it one place, so no need for brevity.
|
This needs a changelog entry, since it changes the AssociativeMemory module interface, right? |
|
Yes it does. |
hunse
force-pushed
the
assoc_mem_reorg
branch
2 times, most recently
from
7583f7c
to
6cec6a3
Compare
|
Okay, I've added a changelog entry and got everything rebased. There were three commits, but the two were very small and the first one already huge, so I just squashed them in. If this looks good to you, @xchoo, I'll merge. |
|
lgtm! 😄 |
- Made the associative memory a nengo.Network rather than just a spa module. - AssociativeMemory module now takes a vocabulary and list of keys to determine the appropriate vectors to use for the underlying AssociativeMemory network.
hunse
force-pushed
the
assoc_mem_reorg
branch
from
6cec6a3
to
f4f831f
Compare
